Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix phpunit-wrapper from resetting variables in global scope in WrapperRunner #540

Merged
merged 4 commits into from
Oct 19, 2020

Conversation

proggga
Copy link
Contributor

@proggga proggga commented Oct 14, 2020

phpunit-wrapper has a problem

it's broken in WrapperRunner
phpunit reset global variables between runs
https://phpunit.readthedocs.io/en/9.3/fixtures.html#global-state

I've got problems with global state in phpunit and this fix helped me.
I tried to user WrapperRunner in my big project, and this helps

this fix prevent variables in phpwrapper (they all in global scope) from turning to null

(Also I removed assert here like in #539, I need this for passing all tests)

@codecov
Copy link

codecov bot commented Oct 14, 2020

Codecov Report

Merging #540 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##              master      #540   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
  Complexity       547       547           
===========================================
  Files             25        25           
  Lines           1703      1703           
===========================================
  Hits            1703      1703           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b9134e1...03ee8ef. Read the comment docs.

Copy link
Member

@Slamdunk Slamdunk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get the idea of the issue, but please can you try to write a test for the bug?
It seems an important one, I'd like to prevent it from being reintroduced by a future refactor

src/Runners/PHPUnit/Runner.php Outdated Show resolved Hide resolved
@proggga
Copy link
Contributor Author

proggga commented Oct 15, 2020

I get the idea of the issue, but please can you try to write a test for the bug?
It seems an important one, I'd like to prevent it from being reintroduced by a future refactor

It's problematic, cause phpunit-wrapper fails, and paratest doing nothing, he is waiting for some command or idk what. It will never complete.

@Slamdunk
Copy link
Member

Slamdunk commented Oct 15, 2020

It's problematic, cause phpunit-wrapper fails, and paratest doing nothing, he is waiting for some command or idk what. It will never complete.

That's ok: the pipeline has timeout-minutes: 3 set, so it will fail if a test is hanging forever.

Please try to write a test so I can see the pipeline failing

@proggga
Copy link
Contributor Author

proggga commented Oct 15, 2020

It's problematic, cause phpunit-wrapper fails, and paratest doing nothing, he is waiting for some command or idk what. It will never complete.

That's ok: the pipeline has timeout-minutes: 3 set, so it will fail if a test is hanging forever.

Please try to write a test so I can see the pipeline failing

Okey, I written test, that fails on master branch (branch without my fix)
problem was in param in phpunit "backupGlobal=true", you can check this out

composer.json Outdated Show resolved Hide resolved
@proggga
Copy link
Contributor Author

proggga commented Oct 15, 2020

Also pls add label after all "hacktoberfest-accepted"

@proggga proggga force-pushed the fix_global_variables branch 2 times, most recently from 62d2b5e to d0d3bf5 Compare October 15, 2020 15:42
@proggga proggga requested a review from Slamdunk October 15, 2020 20:07
@Slamdunk Slamdunk merged commit 2f81f7a into paratestphp:master Oct 19, 2020
@Slamdunk
Copy link
Member

Thank you @proggga

@proggga proggga deleted the fix_global_variables branch October 19, 2020 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants