Repository navigation
Smoke test command from CI - #463
Merged
Merged
Conversation
mgaffigan
requested review from
a team,
NicoPiel,
gibson9583,
jonbartels,
kayyagari,
kpalang,
pacmano1 and
ssrowe
September 26, 2026 00:12
mgaffigan
force-pushed
the
fix/command
branch
from
September 26, 2026 01:48
c9cd706 to
89adc9f
Compare
mgaffigan
force-pushed
the
fix/command
branch
from
September 26, 2026 02:01
89adc9f to
38705e8
Compare
Contributor
Author
|
Related #231 |
Member
I updated this PR as it already had considerable work done on it if we can review that one. It would only replace the first commit from this PR. |
mgaffigan
changed the title
pacmano1
requested changes
Oct 5, 2026
pacmano1
left a comment
pacmano1
left a comment
Contributor
There was a problem hiding this comment.
Two of these tests pass when the CLI fails. I checked against a build of main at 11ad744 (this PR doesn't change the CLI), with an nginx proxy returning 500 for one request at a time:
- A failed
statuspassesrunsReadOnlyCommands.statusprints its header before the request, and aClientExceptionfrom a statement prints as a stack trace, not anError:line. Withchannels/statusesreturning 500, all six assertions passed. RejectingExceptionin the output catches it. - A failed logout passes
logsInAndReportsServerVersion. The CLI printsError: Could not logout from server.and thenDisconnected from server.anyway, so the comment on that assertion is wrong. AnError:check there catches it. - Neither live test asserts exit code 0. That wouldn't catch either failure above, since both exit 0, but it's the code scripts rely on since #231.
pacmano1
approved these changes
Oct 5, 2026
tonygermano
approved these changes
Oct 6, 2026
gibson9583
approved these changes
Oct 6, 2026
Nothing covered command end to end. The harness image now carries the CLI and runs it as a child process, so the distribution layout and launcher manifest are covered too. Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net>
The unreachable-server test expected a ClientException in the output, which the CLI now prints only with -d. Assert the EX_UNAVAILABLE exit code and the error message instead. Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net>
A failed statement prints a stack trace rather than an Error: line, and a failed logout prints Error: before still reporting a disconnect, so neither was caught. Reject Exception and Error: in the output, and assert exit code 0 on the live-server tests. Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net>
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
4 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.
Adds CI smoke tests for
command, which had no end-to-end coverage. The harness image now carries the CLI and runs it as a child process, so the distribution layout and launcher manifest are covered too.Review notes:
exitsWhenServerIsUnreachableis the regression test for the hang fixed in Bug - Fix CLI login error #231 - it fails as a 90s timeout if the hang returns, and asserts exit code 69#280 depends on this.