-
Notifications
You must be signed in to change notification settings - Fork 10
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
feat:checking that the client is allowed to run against staging #124
Conversation
ef50109
to
0b6004c
Compare
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.
LGTM overall, but with one broader design thought: rather than enabling individual tests to be run against staging, maybe we should allow the entire suite to be flipped between production and staging at a time?
My rationale: at least for testcases that do end-to-end signing (meaning they aren't implicitly tied to a specific instance already), we expect them to work on both instances and probably don't want to duplicate the code for both. If the entire suite supports being run in either mode (with skips where necessary for tests that are hard-baked against one or the other), then we get additional coverage against both instances for free.
Thoughts? This may be more effort than is immediately worth 🙂
Signed-off-by: javanlacerda <[email protected]>
0b6004c
to
c53f6c5
Compare
We can try that... but it may make sense to still enable just a subset of tests (the end-to-end ones) in the beginning: otherwise it might be quite a bit of work to re-create all of the test assets for staging (I assume that will be needed at least: I haven't created any of the assets) If all (or many) tests can be run against staging, then we need to provide clients a way to enable/disable staging testing: xfail for individual tests is not viable... Some potential solutions:
I suppose the second one makes more sense? |
Waving hands about what tests should be executed by pytest:
|
Some of results that currently PASS when running |
Yeah, getting the production/staging states right will probably require a larger refactor/discussion of how we want the CI to run the suite. I'm okay with punting on that for now 🙂 |
Signed-off-by: William Woodruff <[email protected]>
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.
LGTM, thank you @javanlacerda!
(CCing @jku for review as well, since I'm now the last pusher.) |
Ah, sorry @jku I merged too quickly here, please feel free to finish your review and suggest any changes if we missed something! |
Should we expect a significant increase in traffic to staging? |
I don't think so -- based on current integrations, this should only run a handful of times a day on the sigstore-python, Java, etc. repos. So the traffic should be roughly the same as the existing staging tests, e.g. the ones |
Closes #121
Summary
One conformance requirement for the clients is that they should be able to run the commands against
staging
. After this PR, the clients should be able to receive the--staging
token in the command line and point to staging resources.Release Note
Updated client run function to receive
staging
argument.Updated run subprocess call to pass
--staging
flag to clients cli when required.Modified CLI parser to receive --staging flag
Update CLI and conformance READMEs adding staging features
Documentation