test(e2e): 239 could pass or skip, never fail - #438
Closed
Sunrisepeak wants to merge 1 commit into
Closed
Conversation
Copy link
Copy Markdown
Member
Sorry, something went wrong.
The skip was decided AFTER the install by pattern-matching the failure text,
and one of the patterns was `*"index"*`. Nearly every mcpp command prints
"package index" somewhere, so EVERY genuine install failure took the skip
branch.
It hid a real one, on the first Windows run where the package was actually
installable:
tar: Cannot connect to C: resolve failed
tar -xf "C:\Users\...\.payloads\Microsoft.VC...vsix" -C "..."
[error] msvc installed but registered none of the programs it declares
GNU tar reads `C:` as a hostname. The install ran for 135 seconds, failed,
and this script printed PASS.
Skip is now decided BEFORE the work, by a positive check for what would make
the test impossible (no msvc row in `toolchain list`). Everything after that
is a failure, and the output is printed rather than folded into a one-line
message.
The recipe-side fix is openxlings/xim-pkgindex#632.
A skip decided by the shape of a failure is not a skip; it is a way of not
looking.
Copy link
Copy Markdown
Member
Author
|
Folded into #440 and closing there. #440 has to re-run its Windows e2e anyway (it was blocked on openxlings/xim-pkgindex#637 / #639), so carrying this as a separate PR would have cost a second full CI cycle for a change to the same file — #440 already touches 239_msvc_managed_toolset.sh. The commit is preserved verbatim, plus two follow-ups that today's run argued for:
|
Sorry, something went wrong.
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
2 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.
Summary
The skip was decided after the install, by pattern-matching the failure text, and one of the patterns was *"index"*. Nearly every mcpp command prints "package index" somewhere, so every genuine install failure took the skip branch.
It hid a real one, on the first Windows run where the package was actually installable:
The install ran for 135 seconds, failed, and this script printed PASS.
Fix
Skip is decided before the work, by a positive check for the thing whose absence would make the test impossible (no msvc row in toolchain list). Everything after that is a failure, and the output is printed rather than folded into a one-line message.
The recipe-side fix for what it hid is openxlings/xim-pkgindex#632 (GNU tar reads C: as a hostname; bsdtar does not, which is why the index's own windows-test passed).
Test plan