Merged
Conversation
… file mutation Problem: When running with destroyOutput set the following problems arise: 1. test/test.js called writeFile(outputFilename, result, options) where result is already a serialized string returned by run(), effectivelly double encoding it. The next test run (`npx jest test/test.js`) would load this corrupted content as the expected snapshot, causing all JSON test cases in test.js to fail. 2. test/__utils__/test-utils.js loadTest() ran the CLI without overriding --output, so the CLI used the output path from options.yaml. Which during a run with `deleteOutput` set, would silently overwrite and mask the corruption from (1) while still producing test failures. Solution: - Write the already-serialized result string directly with `fs.writeFileSync` in test.js. - Pass --output <tmpfile> in loadTest so the CLI writes to a temp file instead of the shared `output`. Output file mutation is now exclusive owned by `test.js`.
Problem: 1. sort and bundle were unconditionally derived from no-sort/no-bundle, ignoring explicit sort/bundle values set directly in options.yaml 2. The no-bundle delete guard incorrectly checked no-sort as its first condition. Solution: Default sort and bundle to true, and only translate the no-X keys when they are actually present in the options file.
Owner
|
@guilhas07 Great of you to figure out the quirky behaviour in the tests suite. Thank you for taking the time to fix it with this PR 👏 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix(test): normalize no-sort/no-bundle option handling in test runner
Problem:
ignoring explicit sort/bundle values set directly in options.yaml
condition.
Solution:
Default sort and bundle to true, and only translate the no-X keys when they
are actually present in the options file.
Test
To test, with the other fix applied, after a
destroyOutput=truerun:npx jest bin/cli.test.js --no-coverage -t "should not bundle reference"fix(test): destroyOutput regeneration double serialization and output file mutation
Problem:
When running with destroyOutput set the following problems arise:
test/test.js called writeFile(outputFilename, result, options) where result is already
a serialized string returned by run(), effectivelly double encoding it.
The next test run (
npx jest test/test.js) would load this corrupted content as theexpected snapshot, causing all JSON test cases in test.js to fail.
test/utils/test-utils.js loadTest() ran the CLI without overriding --output, so the
CLI used the output path from options.yaml. Which during a run with
deleteOutputset, would silently overwrite and maskthe corruption from (1) while still producing test failures.
Solution:
fs.writeFileSyncin test.js.shared
output. Output file mutation is now exclusive owned bytest.js.