-
Notifications
You must be signed in to change notification settings - Fork 55
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: make root tests easier to run #501
test: make root tests easier to run #501
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this, looks very simple. Just two minor tweaks requested.
} | ||
} | ||
|
||
args = append(args, "extra", "arguments", "invalid") | ||
_, err = cli.ParserForTest().ParseArgs(args) | ||
c.Assert(err, check.Equals, cli.ErrExtraArgs) | ||
s.ResetStdStreams() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test case uses a for loop to test combine=false
and combine=true
.
When running as root, adding the unreadableLayer
would succeed and the stdout interferes with the next loop. So, here I added a reset on the stdout so that the two cases don't interfere with each other.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, thanks -- looks good to me!
Background: Previously, when adding a new root test, we had to modify the GitHub Actions workflow to include the newly added test, which is some extra operational overhead.
Per discussion here, we agreed to run all tests with root and skip those tests that would fail with root (checking permissions and stuff), and this PR skips tests that would fail when running with root and made the GitHub Actions workflow for root tests much easier.