Merge ~sbeattie/qa-regression-testing:ecdsautils-lp-ci into qa-regression-testing:master

Proposed by Steve Beattie
Status: Merged
Merge reported by: Steve Beattie
Merged at revision: 5563f8a226839a67f478f35dce2a50adbbccfc83
Proposed branch: ~sbeattie/qa-regression-testing:ecdsautils-lp-ci
Merge into: qa-regression-testing:master
Diff against target: 234 lines (+102/-41)
2 files modified
.launchpad.yaml (+14/-0)
scripts/test-ecdsautils.py (+88/-41)
Reviewer Review Type Date Requested Status
Alex Murray (community) Approve
Review via email: mp+448427@code.staging.launchpad.net

Commit message

test-ecdsautils: misc improvements and enable in LP CI

Steve Beattie (5):
      * [062bea06] test-ecdsautils.py: convert ecdsaTest to use subTests for each test vector
      * [32aea5d4] test-ecdsautils.py: multiple improvements
      * [e1613441] test-ecdsautils.py: add a couple more signature boundary condition tests
      * [1e837ae0] .launchpad.yaml: enable ecdsautils test in LP CI
      * [35b4abe2] .launchpad.yaml: disable test-ecdsautils in LP CI for bionic

Description of the change

test-ecdsautils: misc improvements and enable in LP CI

Adjust tests to make use of the subtests infrastructure, as well as use more the testlib command infrastructure to ensure that error codes are caught and trigger test failures. Also add a couple more of boundary condition checks similar to the test for CVE-2022-24884.

Finally, and the primary point I'd like to have a review / discussion on, is to enable running the tests in LP CI, but disabled for bionic becuase the CVE-2022-24884 is only available in the Ubuntu Pro archive, and so the tests are guaranteed to fail there. I'm not convinced this is the right approach to handle this. As bionic's time in ESM status grows, this is only going to be more of a problem, so we should come up with a coherent process for this, as well as for things where fixes only live in the Pro Apps archive.

To post a comment you must log in.
Revision history for this message
Alex Murray (alexmurray) wrote :

Apologies for not looking at this earlier - the issue regarding tests that will fail due to lack of ESM is tricky but I am not sure it should block merging of this - until we have a solution I think it makes sense to have it commented out since at least getting this merged for the other releases adds good value.

+1 from me.

review: Approve
Revision history for this message
Steve Beattie (sbeattie) wrote :

Thanks. I've changed my mind on this slightly, I think it's worth keeping the tests running on bionic, but expecting that the test will fail in bionic when run in the LP CI infrastructure, which I'm currently testing before pushing to master.

An alternative would be to add some infrstructure in testlib that can be used to identify if the environment has Ubuntu Pro enabled and set test expectations accordingly, but there would need to be smarts around running tests while developing fixes/preparing an update, before the fix has landed in Pro, so that it doesn't expect failure when run within uvt, for example.

Revision history for this message
Steve Beattie (sbeattie) wrote :

Merged in https://git.launchpad.net/qa-regression-testing/commit/?id=25dbb3e7a8c756c51890abaa7b26e471aef6a60a (due to rebasing my merge commit, this MR did not automatically get closed, marking as merged manually).

There was an error fetching revisions from git servers. Please try again in a few minutes. If the problem persists, contact Launchpad support.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches