This repository was archived by the owner on Aug 31, 2023. It is now read-only.
fix(rome_cli): normalize the content of CLI snapshots #3835
Conversation
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
MichaReiser
approved these changes
Nov 24, 2022
| match $result { | ||
| Ok(res) => res, | ||
| Err(err) if option_env!("CI") == Some("1") => panic!("failed to create symlink: {err}"), | ||
| Err(_) => return, |
There was a problem hiding this comment.
I would find it useful if the macro could write a message explaining that the test failed with the given message and then give instructions how to run that test if necessary (run as administrator)
There was a problem hiding this comment.
When not running on CI the test will not fail here, it will be reported as a success instead. Ideally it would be reported as skipped but ignoring tests dynamically is not supported by the default Rust test harness. Printing a message wouldn't work either since the test framework also captures the output of tests and only prints it to the console if the test fails.
ematipico
approved these changes
Nov 24, 2022
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Summary
This PR is an attempt at improving the general stability of CLI snapshot tests by "redacting" some unstable elements from the snapshot content that would be susceptible to change from one run to another. This includes the path to the current executable and the temporary directory, the time it takes for the CLI to execute, and the platform-specific path separator.
Test Plan
This should solve some of the stability issues we currently have with the CI runs, but not all of them though: we still have semi-random failures on the
max_diagnosticstests that can be reproduced more easily on some platforms than other.