search

open-quantum-safe / tsc

OQS Technical Steering Committee resources
https://openquantumsafe.org/
Creative Commons Attribution 4.0 International
3 stars 5 forks source link

CI in OQS: guidelines for responsible use #5

Open bhess opened 5 months ago

bhess commented 5 months ago

This issue is to suggest setting guidelines in OQS for resource-responsible use of CI.

OQS so far already did much more than "typical projects" in this regards/not blindly running CI in the various projects -- but there's still quite some way to go: What about OQS "institutes" this? Ask contributors to use "[skip ci]" as often as possible, do (CI) spot checks before doing "heavy lifting", possibly move more CI to the weekly tests, reduce the repetition of the profiling runs, etc?

Originally posted by @baentsch in https://github.com/open-quantum-safe/oqs-provider/issues/376#issuecomment-2006940977

I agree with the above and think some guidelines would help to avoid redundant CI runs, without hurting the overall test quality. This is not necessarily motivated by a lack of available resources (the LF indicated that more Github Actions resources can be made available to the projects, which I think is a good thing), but rather by the desire to make most efficient use of the available resources.

Any feedback welcome.

thb-sb commented 5 months ago

I think this is a good idea.

One thing we could do to pave the way is to smartly list all files that require tests to be run (basically *.c, *.h, and some generated .md files?). If the changes don't involve at least one of these files, then nothing would be run.

SWilson4 commented 5 months ago

I agree that this is a good idea. One "obvious" way to make CI more efficient would be to eliminate redundant runs (e.g., the same workflow being triggered by both "push" and "pull_request" on some PRs). I haven't yet found a clean way to do this, but perhaps somebody with more GitHub-specific expertise might.

IMO this is a great place to contribute for those who want to help out OQS / PQCA but don't feel comfortable working with cryptographic code.

dstebila commented 5 months ago

I agree that this is a good idea. One "obvious" way to make CI more efficient would be to eliminate redundant runs (e.g., the same workflow being triggered by both "push" and "pull_request" on some PRs). I haven't yet found a clean way to do this, but perhaps somebody with more GitHub-specific expertise might.

@ryjones might have experience with this.

ryjones commented 5 months ago

@SWilson4 do you mean on PR and merge, or the same job runs on both PR and the push of the PR to a branch?

SWilson4 commented 5 months ago

@SWilson4 do you mean on PR and merge, or the same job runs on both PR and the push of the PR to a branch?

The latter. For example, see the "Linux and macOS tests" here: https://github.com/open-quantum-safe/liboqs/pull/1725/checks. These tests (and all others triggered to run on both push and pull_request events run twice on every push to an existing PR (at least for my PRs and presumably non-fork PRs).

ryjones commented 5 months ago

@SWilson4 this stack overflow answer shows the way.

  push:
    branches:
      - 'main'
      - 'releases/**'

might be better

ryjones commented 5 months ago

take a look here

dstebila commented 5 months ago

The outcome of today's OQS status call was that it would be good to have a document setting some guidelines / documenting best practices, saved as a Markdown file in the TSC repository. @bhess and @thb-sb to kick it off.

baentsch commented 5 months ago

Thanks very much for documenting decisions such as this, @dstebila .

May I augment with the suggestion to link the file thus generated from a (new) "getting started contributing to OQS" write-up at www.openquantumsafe.org? Maybe off/in/at https://openquantumsafe.org/about/#getting-started (?)

SWilson4 commented 1 week ago

In the absence of codified guidelines, here are some principles I tried to follow in https://github.com/open-quantum-safe/liboqs/pull/1909. I would suggest that we work toward following these in other projects as well.

  1. Cancel in-progress jobs on a double-push. This can be achieved using concurrency groups.
  2. Only run resource-heavy tests if basic checks pass first.
  3. Run only a minimal set of tests automatically on push to non-main branches.
  4. As a follow-up to the above, give developers the ability to trigger heavier tests manually—ideally both by pushing a button in the GitHub UI (using workflow dispatches) and by including a keyword in the commit message.

I found reusable workflows to be very helpful for implementing all of these. These allow us to create "caller" workflows for each condition (push, pull request, release, schedule, etc) which trigger a subset of separate "callee" workflows (macOS tests, Linux tests, downstream tests, extended tests, etc). With this pattern we can get very fine-grained control over which tests run and when.