fix(cli): tolerate #2702 stderr noise #2704
Open
Conversation
This file contains 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
erights
changed the base branch from
master
to
markm-fix2700-ignore-safe-asynchook-extras
erights
changed the title
This was referenced Jan 27, 2025
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.
Staged on #2701
Closes: #2702
Refs: #XXXX
Description
As shown by #2702:
When tests are run under the vscode JavaScript Debug Terminal, every time that test spawns a subshell, extra noise appears on the stderr of that subshell, such as
This seems symptomatic of a widespread assumption that other tools on the toolchain can inject extra output into the stderr of subshells without typically causing problems.
The test framework code changed by this PR had defaulted both an absent stdout expectation and an absent stderr expectation to expecting the empty string. This PR changes the stderr default expectation to an regexp that accepts anything. This can still be overridden by an explicitly provided stderr expectation, including one explicitly expecting an empty string, so there is no loss of generality. Just a change in the default.
Security Considerations
This change will cause some tests to pass that might have otherwise failed. Some of these masked failures may have been symptoms of actual problems, rather than tool generated noise. This PR will mask such problems.
Scaling Considerations
none
Documentation Considerations
none
Testing Considerations
Because the #2702 bug this PR fixes is about a behavior seen under the vscode JavaScript Debug Terminal, I didn't see a way to write an automated test to check the fix under CI. Rather, I just verified the fix locally and interactively using vscode.
Compatibility Considerations
As mentioned in Security Considerations above, this PR may mask genuine errors that would have been flagged prior to this PR.
Upgrade Considerations
none