Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[Config] Fix GeneratedConfigTest not being able to generate snapshots anymore#61140
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:7.2
Are you sure you want to change the base?
Conversation
Psalm errors seem unrelated to my changes |
| unlink($outputDir); | ||
| mkdir($outputDir); | ||
| $this->tempDir[] = $outputDir; | ||
| if (null === $outputDir) { |
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.
$outputDir is going to be always null
what we need is probably
| if (null ===$outputDir) { | |
| if (1 <\func_num_args()) { |
KevinVanSonsbeekJul 17, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
Lines 87 -> 89 of the test contain commented code, that when uncommented try to regenerate the snapshot files.
On line 88 it specifically passes the directory of the expected code, so the new snapshots are generated in the given location. Resulting in the outputDir not being null.
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.
got it thanks
| unlink($outputDir); | ||
| mkdir($outputDir); | ||
| $this->tempDir[] = $outputDir; | ||
| if (null === $outputDir) { |
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.
got it thanks
Uh oh!
There was an error while loading.Please reload this page.
PR#57614 Removed multiple uses of
uniqid. In the specific PR changes were made to the GeneratedConfigTest, to change how the directory was built for the config.This inadvertedly broke the generating of snapshot files, due to the outputDir being changed to a by reference argument, and always overwriting the value to a temp dir. (While the snapshot files should not be written to a temp dir)