Skip to content

Commit

Permalink
Reconfigure CI pipelines for PR and merge queue (Qiskit#11526)
Browse files Browse the repository at this point in the history
* Reconfigure CI pipelines for PR and merge queue

This commit refactors the pipeline organisation of our CI to reduce the
total number of jobs run on any given PR merge, and to reduce the
critical path length of merging.  In particular:

- the lint, docs and QPY tests are combined into one job for a single CI
  worker.  These three jobs individually add up to less than a test run,
  and moving QPY from the first-stage PR test run to this rebalances the
  (now) two jobs in the stage to be more equal in runtime.

- a new pipeline is added specifically for the merge queue.  Previously
  this reused the same two-stage PR pipeline.  The two-stage system
  unnecessarily lengthened the critical path, as when a PR enters the
  merge queue, it as already passed PR CI, and so is highly likely to
  pass all jobs.  In addition to flattening to a single stage, one each
  of the macOS and Windows jobs are removed to lower the amount of VMs
  needed and reduce the chances of a timeout (these OSes are more likely
  to get a dodgy VM and time out than Linux).  The only way a new test
  failure should appear (other than a flaky test) is by a logical merge
  conflict, which would be quite unlikely to only affect a particular
  Python version.

To make it easier to run the lint and docs jobs together, and ensure
that both run even if there's a failure in one, the full lint
configuration is merged back to `tox.ini`, and that's used to do the
linting.  This makes it more consistent for developers, as well.

* Allow cargo as an external in 'tox'

* Add display name to lint job

* Use editable install for tox lint job

The lint job will fail unless the Rust components are installed into the
source directory, since that's where `pylint` will look for them to
resolve the imports.

* Fix typo in merge-queue stage condition
  • Loading branch information
jakelishman authored Jan 17, 2024
1 parent 27761bd commit ed79d42
Show file tree
Hide file tree
Showing 5 changed files with 97 additions and 110 deletions.
62 changes: 0 additions & 62 deletions .azure/lint-linux.yml

This file was deleted.

37 changes: 29 additions & 8 deletions .azure/docs-linux.yml → .azure/lint_docs_qpy-linux.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ parameters:
displayName: "Version of Python to use"

jobs:
- job: 'Docs'
- job: 'Lint_Docs_QPY'
displayName: 'Lint, documentation and QPY'
pool: {vmImage: 'ubuntu-latest'}

variables:
Expand All @@ -20,14 +21,13 @@ jobs:
displayName: 'Use Python ${{ parameters.pythonVersion }}'

- bash: tools/install_ubuntu_docs_dependencies.sh
displayName: 'Install dependencies'
displayName: 'Install docs dependencies'

- bash: |
set -e
tox -e docs
# Clean up Sphinx detritus.
rm -rf docs/_build/html/{.doctrees,.buildinfo}
displayName: 'Run Docs build'
- bash: tox run -e docs,lint
displayName: 'Lint and docs'

- bash: rm -rf docs/_build/html/{.doctrees,.buildinfo}
displayName: 'Clean up docs detritus'

- task: ArchiveFiles@2
inputs:
Expand All @@ -43,3 +43,24 @@ jobs:
artifactName: 'html_docs'
Parallel: true
ParallelCount: 8

- task: Cache@2
inputs:
key: 'qpy | test/qpy_compat/test_qpy.py | "$(Build.BuildNumber)"'
restoreKeys: |
qpy | test/qpy_compat/test_qpy.py
path: qpy_files
displayName: Cache old QPY files

- bash: |
set -e
# Reuse the docs environment to avoid needing to rebuild another
# version of Qiskit.
source .tox/docs/bin/activate
mv qpy_files/* test/qpy_compat || :
pushd test/qpy_compat
./run_tests.sh
popd
mkdir qpy_files || :
mv test/qpy_compat/qpy_* qpy_files/.
displayName: 'Run QPY backwards compat tests'
21 changes: 0 additions & 21 deletions .azure/test-linux.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,6 @@ parameters:
- name: "testRust"
type: boolean

- name: "testQPY"
type: boolean

- name: "testImages"
type: boolean

Expand Down Expand Up @@ -166,24 +163,6 @@ jobs:
displayName: 'Publish images on test failure'
condition: failed()

- ${{ if eq(parameters.testQPY, true) }}:
- task: Cache@2
inputs:
key: 'qpy | test/qpy_compat/test_qpy.py | "$(Build.BuildNumber)"'
restoreKeys: |
qpy | test/qpy_compat/test_qpy.py
path: qpy_files
displayName: cache old qpy
- bash: |
set -e
mv qpy_files/* test/qpy_compat || :
pushd test/qpy_compat
./run_tests.sh
popd
mkdir qpy_files || :
mv test/qpy_compat/qpy_* qpy_files/.
displayName: 'Run QPY backwards compat tests'
- ${{ if eq(parameters.testImages, true) }}:
- bash: |
set -e
Expand Down
76 changes: 59 additions & 17 deletions azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@ pr:
include:
- '*'

# A schedule only runs on branches that match include rules from _both_ `main`
# and the branch itself. On `main`, we blanket include all branches that might
# want to be enabled, then particular branches can override it to exclude
# themselves by removing themselves from the trigger list. For example, old
# stable branches can remove `stable/*` from their copy of this file once they
# reach their end-of-life.
schedules:
- cron: "20 6 * * *"
displayName: "Complete matrix test"
Expand Down Expand Up @@ -82,9 +88,13 @@ parameters:
# - push to a pull request (`PullRequest`)
# For `IndividualCI` (push/merge to a branch/tag on Qiskit/qiskit-terra), you
# need to examine `variables['Build.SourceBranch']` to determine whether it's a
# branch or a tag.
# branch or a tag, and if a branch, then whether it's in the merge queue or a
# push to a "real" branch.
stages:
# Nightly cron job.
#
# For this to run on a branch, the `schedules` trigger up at the top of this
# file needs to match on _both_ `main` and the branch itself.
- ${{ if eq(variables['Build.Reason'], 'Schedule') }}:
- stage: "Nightly"
displayName: "Nightly complete matrix tests"
Expand All @@ -93,7 +103,6 @@ stages:
- template: ".azure/test-linux.yml"
parameters:
pythonVersion: ${{ version }}
testQPY: false
testImages: false
testRust: false
installOptionals: true
Expand Down Expand Up @@ -123,10 +132,10 @@ stages:
id: 7864
comment: Nightly test job failed at commit $(Build.SourceVersion). View the logs at $(System.TeamFoundationCollectionUri)$(System.TeamProject)/_build/results?buildId=$(Build.BuildId).

# Full PR suite. This also needs to apply to the merge queue, which appears
# as a push event. The queue won't get cron triggered because the schedule
# trigger doesn't have it as a target.
- ${{ if or(eq(variables['Build.Reason'], 'PullRequest'), contains(variables['Build.SourceBranch'], 'gh-readonly-queue')) }}:
# Full PR suite. PRs need to pass this pipeline in order to be moved to the
# merge queue, where they'll use the next rule as the branch-protection rule
# for the final merge to the base branch.
- ${{ if eq(variables['Build.Reason'], 'PullRequest') }}:
# The preliminary stage should be small in both total runtime (including
# provisioning) and resources required. About half of PR commits result in
# a CI failure, and over 90% of these are in linting, documention or a test
Expand All @@ -135,14 +144,10 @@ stages:
- stage: "Lint_Docs_Prelim_Tests"
displayName: "Preliminary tests"
jobs:
- template: ".azure/lint-linux.yml"
- template: ".azure/lint_docs_qpy-linux.yml"
parameters:
pythonVersion: ${{ parameters.minimumPythonVersion }}

- template: ".azure/docs-linux.yml"
parameters:
pythonVersion: ${{ parameters.documentationPythonVersion }}

- template: ".azure/test-linux.yml"
parameters:
pythonVersion: ${{ parameters.minimumPythonVersion }}
Expand All @@ -153,13 +158,12 @@ stages:
# to fail fast in the first CI stage.
installOptionals: true
testRust: true
testQPY: true
testImages: true

# The rest of the PR pipeline is to test the oldest and newest supported
# versions of Python. It's very rare for a failure to be specific to an intermediate version of
# Python, so we just catch those in the cron-job pipeline to reduce the
# amount of resources used.
# versions of Python. It's very rare for a failure to be specific to an
# intermediate version of Python, so we just catch those in the cron-job
# pipeline to reduce the amount of resources used.
- stage: "Tests"
displayName: "Main tests"
dependsOn: "Lint_Docs_Prelim_Tests"
Expand All @@ -168,7 +172,6 @@ stages:
parameters:
pythonVersion: ${{ parameters.maximumPythonVersion }}
testRust: false
testQPY: false
testImages: false
installFromSdist: true
installOptionals: false
Expand All @@ -193,6 +196,46 @@ stages:
pythonVersion: ${{ parameters.maximumPythonVersion }}
installOptionals: false

# Merge queue. A PR that reaches here has already passed the more rigorous PR
# suite, so is very likely to pass. The main reasons for failures here are
# flaky VMs timing out (which we can't do much about), or a merge conflict
# with another PR that is also in the merge queue.
#
# There's no reason to have multiple stages in this case, because we're
# expecting it to pass. Having more than one stage frustrates parallel
# throughput in low-contention cases, and guarantees a longer critical path.
- ${{ if and(eq(variables['Build.Reason'], 'IndividualCI'), contains(variables['Build.SourceBranch'], 'gh-readonly-queue')) }}:
- stage: "Merge_Queue"
displayName: "Merge queue"
jobs:
- template: ".azure/lint_docs_qpy-linux.yml"
parameters:
pythonVersion: ${{ parameters.minimumPythonVersion }}

- template: ".azure/test-linux.yml"
parameters:
pythonVersion: ${{ parameters.minimumPythonVersion }}
installOptionals: true
testRust: true
testImages: true

- template: ".azure/test-linux.yml"
parameters:
pythonVersion: ${{ parameters.maximumPythonVersion }}
installOptionals: false
testRust: false
testImages: false

- template: ".azure/test-macos.yml"
parameters:
pythonVersion: ${{ parameters.maximumPythonVersion }}
installOptionals: false

- template: ".azure/test-windows.yml"
parameters:
pythonVersion: ${{ parameters.maximumPythonVersion }}
installOptionals: false

# Push to main or the stable branches. The triggering branches also need to
# be in the triggers at the top of this file.
- ${{ if and(eq(variables['Build.Reason'], 'IndividualCI'), or(startsWith(variables['Build.SourceBranch'], 'refs/heads/main'), startsWith(variables['Build.SourceBranch'], 'refs/heads/stable/'))) }}:
Expand All @@ -202,6 +245,5 @@ stages:
parameters:
pythonVersion: ${{ parameters.branchPushPythonVersion }}
testRust: true
testQPY: true
testImages: true
installOptionals: false
11 changes: 9 additions & 2 deletions tox.ini
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[tox]
minversion = 3.3.0
minversion = 4.0
envlist = py38, py39, py310, py311, py312, lint-incr
isolated_build = true

Expand All @@ -23,9 +23,16 @@ commands =

[testenv:lint]
basepython = python3
# `pylint` will examine the source code, not the version that would otherwise be
# installed in `site-packages`, so we use an editable install to make sure the
# compiled modules are built into a valid place for it to find them.
package = editable
allowlist_externals = cargo
commands =
ruff check qiskit test tools examples setup.py
black --check {posargs} qiskit test tools examples setup.py
cargo fmt --check
ruff check qiskit test tools examples setup.py
cargo clippy -- -D warnings
pylint -rn qiskit test tools
# This line is commented out until #6649 merges. We can't run this currently
# via tox because tox doesn't support globbing
Expand Down

0 comments on commit ed79d42

Please sign in to comment.