-
Notifications
You must be signed in to change notification settings - Fork 41
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
workflows: Add conformance testing workflow #298
Conversation
Signed-off-by: Alex Cameron <asc@tetsuo.sh>
23531ea
to
2b1be7b
Compare
Signed-off-by: Alex Cameron <asc@tetsuo.sh>
f1ed0e9
to
57a6b50
Compare
.github/sigstore-python-conformance
Outdated
#!/usr/bin/env python3 | ||
|
||
""" | ||
A wrapper to convert `sigstore-conformance` CLI protocol invocations to match `sigstore-python`. |
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.
Not really sure where the right spot for this is.
It doesn't seem like it belongs in the actual package itself. But on the other hand, I want to access this wrapper from the sigstore-conformance
tests so that when we iterate on the action, we have visibility when we're breaking basic things.
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.
IMO test/integration/sigstore-python-conformance
would be a reasonable path -- that would reuse our existing test structure but shouldn't interfere with pytest
.
If it does, we could move our current unit tests from test/
to test/unit/
.
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.
Good idea. I think that's definitely cleaner.
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.
.github/sigstore-python-conformance
Outdated
#!/usr/bin/env python3 | ||
|
||
""" | ||
A wrapper to convert `sigstore-conformance` CLI protocol invocations to match `sigstore-python`. |
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.
Separate tiny nit: could we expand this comment to include a link to the sigstore-conformance
repo?
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.
Signed-off-by: Alex Cameron <asc@tetsuo.sh>
c94d51a
to
5637528
Compare
Signed-off-by: Alex Cameron <asc@tetsuo.sh>
Signed-off-by: Alex Cameron <asc@tetsuo.sh>
Signed-off-by: Alex Cameron <asc@tetsuo.sh>
.github/workflows/conformance.yml
Outdated
- uses: actions/setup-python@7f80679172b057fc5e90d70d197929d454754a5a | ||
- name: install sigstore-python | ||
run: python -m pip install . | ||
- uses: trailofbits/sigstore-conformance@v0.0.1 |
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.
nit: could use hash pinning like the other actions do -- dependabot* should still follow your latest release.
*) just realized dependabot does not seem to be set up for GH actions currently: will file an issue
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.
Signed-off-by: Alex Cameron <asc@tetsuo.sh>
Github is currently censoring me: leaving a review doesn't do anything... So here it is as a comment: (Github ate my original review: apologies if this is a duplicate) Looks good to me. The brutally simple test scaffold seems like a good start: it may be too simplistic but this should be a good way to find out. Can you verify the effects of |
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!
Closes #297