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 FileLoader.php fatal error when loading a non existing file #3508

Closed
wants to merge 3 commits into from

Conversation

mkveksas
Copy link

@mkveksas mkveksas commented Feb 3, 2019

No description provided.

@mkveksas mkveksas changed the title Fix/file loader Fix FileLoader.php fatal error when loading a non existing file Feb 3, 2019
@codecov
Copy link

codecov bot commented Feb 3, 2019

Codecov Report

Merging #3508 into master will decrease coverage by <.01%.
The diff coverage is 66.66%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #3508      +/-   ##
============================================
- Coverage     82.77%   82.77%   -0.01%     
- Complexity     3679     3680       +1     
============================================
  Files           144      144              
  Lines          9593     9595       +2     
============================================
+ Hits           7941     7942       +1     
- Misses         1652     1653       +1
Impacted Files Coverage Δ Complexity Δ
src/Util/FileLoader.php 94.73% <66.66%> (-5.27%) 8 <0> (+1)

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 000b6c8...1451869. Read the comment docs.

@sebastianbergmann
Copy link
Owner

Pull requests for bug fixes should be made to the oldest branch that is supported and has the problem you are trying to fix.

That being said, can you share an example that triggers the problem you are trying to fix? Thanks!

@mkveksas
Copy link
Author

mkveksas commented Feb 3, 2019

Pull requests for bug fixes should be made to the oldest branch that is supported and has the problem you are trying to fix.

That being said, can you share an example that triggers the problem you are trying to fix? Thanks!

Sure thing! Running the Composer example from https://phpunit.de/getting-started/phpunit-8.html by trying to load a non existing test:

./vendor/bin/phpunit --bootstrap vendor/autoload.php tests/NonExistingTest

output:

PHP Fatal error:  Uncaught TypeError: fopen() expects parameter 1 to be a valid path, boolean given in /home/projects/test-project/vendor/phpunit/phpunit/src/Util/FileLoader.php:37
Stack trace:
#0 /home/projects/test-project/vendor/phpunit/phpunit/src/Util/FileLoader.php(37): fopen(false, 'r')
#1 /home/projects/test-project/vendor/phpunit/phpunit/src/Runner/StandardTestSuiteLoader.php(40): PHPUnit\Util\FileLoader::checkAndLoad('tests/NonExisti...')
#2 /home/projects/test-project/vendor/phpunit/phpunit/src/Runner/BaseTestRunner.php(131): PHPUnit\Runner\StandardTestSuiteLoader->load('tests/NonExisti...', 'tests/NonExisti...')
#3 /home/projects/test-project/vendor/phpunit/phpunit/src/Runner/BaseTestRunner.php(79): PHPUnit\Runner\BaseTestRunner->loadSuiteClass('tests/NonExisti...', '')
#4 /home/projects/test-project/vendor/phpunit/phpunit/src/TextUI/Command.php(185): PHPUnit\Runner\BaseTestRunner->getTest('tests/NonExisti...', '', Array)
#5 /home/projects/test-project/vendor/phpunit/phpunit/src/TextUI/Command. in /home/projects/test-project/vendor/phpunit/phpunit/src/Util/FileLoader.php on line 37

Happy to change the merge request into the branch that has the problem, I believe 8.0 ? Thanks!

@sebastianbergmann
Copy link
Owner

Thank you for your contribution. I appreciate the time you invested in preparing this pull request. However, I have decided not to merge it. Instead of checking $includePathFilename !== false I wanted to ensure that $includePathFilename can only contain a string.

@mkveksas
Copy link
Author

mkveksas commented Feb 4, 2019

No worries, hope to find and fix more bugs in the future!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants