Skip to content

[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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: 7.2
Choose a base branch
from

Conversation

KevinVanSonsbeek
Copy link
Contributor

@KevinVanSonsbeek KevinVanSonsbeek commented Jul 16, 2025

Q A
Branch? 7.2
Bug fix? no
New feature? no
Deprecations? no
Issues Fix #61139
License MIT

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)

@KevinVanSonsbeek
Copy link
Contributor Author

Psalm errors seem unrelated to my changes

@KevinVanSonsbeek KevinVanSonsbeek changed the title bugfix(#61139): Only generate an outputDir if none is set. bugfix(#61139): Fix GeneratedConfigTest not being able to generate snapshots anymore Jul 16, 2025
@carsonbot carsonbot changed the title bugfix(#61139): Fix GeneratedConfigTest not being able to generate snapshots anymore [Config] bugfix(#61139): Fix GeneratedConfigTest not being able to generate snapshots anymore Jul 17, 2025
@nicolas-grekas nicolas-grekas changed the title [Config] bugfix(#61139): Fix GeneratedConfigTest not being able to generate snapshots anymore [Config] Fix GeneratedConfigTest not being able to generate snapshots anymore Jul 17, 2025
@nicolas-grekas nicolas-grekas requested a review from GromNaN July 17, 2025 06:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants