-
-
Notifications
You must be signed in to change notification settings - Fork 427
Fix(testing): Register shutdown handler to delete temp input file #7800
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
base: main
Are you sure you want to change the base?
Conversation
guanguans
commented
Jan 3, 2026
- Register shutdown function to delete the temporary input file via FileSystem::delete
- Avoid leftover test artifacts by removing fixture-derived files on process exit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR addresses a testing cleanup issue where temporary input files are not deleted when debugging statements like dd(), die(), or exit() prevent the normal execution of PHPUnit's tearDown() method. The solution registers a shutdown function to ensure file deletion even when the test process terminates abnormally.
Key Changes
- Registers a shutdown handler in
createInputFilePathto delete temporary test input files - Ensures cleanup occurs even when
dieorexitstatements bypasstearDown()
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| $fixtureBasename = pathinfo($trimmedFixtureFilePath, PATHINFO_BASENAME); | ||
| return $inputFileDirectory . '/' . $fixtureBasename; | ||
| $inputFilePath = $inputFileDirectory . '/' . $fixtureBasename; | ||
| register_shutdown_function([FileSystem::class, 'delete'], $inputFilePath); |
Copilot
AI
Jan 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Registering a shutdown function for every test execution will accumulate multiple shutdown handlers throughout the test run. Each test that calls doTestFile will register a new shutdown function, and all of them will attempt to execute at process termination.
Additionally, when tearDown executes normally, it already deletes the temporary file. The shutdown function will then attempt to delete an already-deleted file, which may cause warnings or errors depending on how Nette\Utils\FileSystem::delete handles non-existent files.
A better approach would be to:
- Check if the file exists before attempting deletion in the shutdown function
- Or, use register_shutdown_function only in setUp/constructor and track files to delete, or
- Consider using a static property to track whether a shutdown handler has already been registered globally, registering it only once to clean up any remaining files
| $fixtureBasename = pathinfo($trimmedFixtureFilePath, PATHINFO_BASENAME); | ||
| return $inputFileDirectory . '/' . $fixtureBasename; | ||
| $inputFilePath = $inputFileDirectory . '/' . $fixtureBasename; | ||
| register_shutdown_function([FileSystem::class, 'delete'], $inputFilePath); |
Copilot
AI
Jan 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The shutdown function approach registers a handler for each test execution, which could lead to performance degradation in large test suites. Consider alternative approaches that are more efficient:
- Use a static collection to track all temporary files and register a single shutdown function once
- Override the PHPUnit tearDownAfterClass method to ensure cleanup happens at the class level
- Use a temporary directory that gets cleaned up once after all tests
The current implementation may result in hundreds or thousands of registered shutdown functions for large test suites, each attempting to delete a file that was likely already cleaned up by tearDown.
5035316 to
f200aeb
Compare