Update logic when stderr is not the console#3458
Merged
Conversation
Previously, cli-test would, by default, check that a stderr output is strictly identical to a saved outcome. When there was no instructions on how to interpret stderr, it would default to requiring it to be empty. There are many tests cases though where stderr content doesn't matter, and we are mainly interested in the return code of the cli. For these cases, it was possible to set a .ignore document, which would instruct to ignore stderr content. This PR update the logic, to make .ignore the default. When willing to check that stderr content is empty, one must now add an empty .strict file. This will allow status message to evolve without triggering many cli-tests errors. This is especially important when some of these status include compression results, which may change as a result of compression optimizations. It also makes it easier to add new tests which only care about the CLI's return code.
but keep warnings and final operation statement. updated tests/cli-tests/ accordingly
Cyan4973
force-pushed
the
stderr_finalStatus
branch
from
a089197 to
82ca008
Compare
terrelln
approved these changes
Jan 27, 2023
Merged
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
3 participants
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.
When
stderris not the console, it's likely a log file or device.In which case, interactive progress updates become a nuisance : instead of updating the same visual line as an activity signal before being overwritten by final operation summary, all these updates are appended into logs, which is rather messy.
Therefore, it's preferable to disable progress updates in these situations.
This was achieved a long time ago, by reducing the
displayLevelto 1 in these cases, which is equivalent to-q.Problem is, this would not only disable progress updates, but also all warnings, and the final operation summary.
In short, it was silencing too much.
This happened because back then we had no other tool than
displayLevel.But today, we can selectively disable progress updates, without impacting other messages.
Change the policy when
stderris not the console : only disable progress updates, but keep final operation summary (and warnings if they happen).Updated
tests/cli-tests/accordingly.Also : changed default policy of
tests/cli-tests/: when no.stderr.strictnorstderr.globare provided, default to ignorestderroutput.Previous policy was requiring
stderrto be empty in this case. If it's important forstderrto be empty to pass the test, provide an empty.stderr.strictfile.