From 39caa8da12b324556d5d5de804ebce210f9caf9f Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Wed, 31 Jan 2024 03:48:04 +0000 Subject: [PATCH] Reconfigure CI pipelines for PR and merge queue (backport #11526) (#11586) * Reconfigure CI pipelines for PR and merge queue (#11526) * 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 (cherry picked from commit ed79d42d6a6ca356bf24aa0f5ca0cb840b672ddd) # Conflicts: # .azure/lint-linux.yml # tox.ini * Fix conflict --------- Co-authored-by: Jake Lishman --- ...docs-linux.yml => lint_docs_qpy-linux.yml} | 37 +++++++-- .azure/test-linux.yml | 21 ----- azure-pipelines.yml | 76 ++++++++++++++----- tox.ini | 11 ++- 4 files changed, 97 insertions(+), 48 deletions(-) rename .azure/{docs-linux.yml => lint_docs_qpy-linux.yml} (51%) diff --git a/.azure/docs-linux.yml b/.azure/lint_docs_qpy-linux.yml similarity index 51% rename from .azure/docs-linux.yml rename to .azure/lint_docs_qpy-linux.yml index a44722bd0c38..99d2b7dc41c9 100644 --- a/.azure/docs-linux.yml +++ b/.azure/lint_docs_qpy-linux.yml @@ -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: @@ -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: @@ -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' diff --git a/.azure/test-linux.yml b/.azure/test-linux.yml index eb456f8497ad..f75170914e65 100644 --- a/.azure/test-linux.yml +++ b/.azure/test-linux.yml @@ -6,9 +6,6 @@ parameters: - name: "testRust" type: boolean - - name: "testQPY" - type: boolean - - name: "testImages" type: boolean @@ -168,24 +165,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 diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 58370138160a..c3154abf412c 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -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" @@ -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" @@ -93,7 +103,6 @@ stages: - template: ".azure/test-linux.yml" parameters: pythonVersion: ${{ version }} - testQPY: false testImages: false testRust: false installOptionals: true @@ -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 @@ -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 }} @@ -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" @@ -168,7 +172,6 @@ stages: parameters: pythonVersion: ${{ parameters.maximumPythonVersion }} testRust: false - testQPY: false testImages: false installFromSdist: true installOptionals: false @@ -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/'))) }}: @@ -202,6 +245,5 @@ stages: parameters: pythonVersion: ${{ parameters.branchPushPythonVersion }} testRust: true - testQPY: true testImages: true installOptionals: false diff --git a/tox.ini b/tox.ini index d5a32e4c0eb4..4a8db99a168c 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -minversion = 3.3.0 +minversion = 4.0 envlist = py38, py39, py310, py311, py312, lint-incr isolated_build = true @@ -29,9 +29,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 qiskit_pkg black --check {posargs} qiskit test tools examples setup.py qiskit_pkg + cargo fmt --check + ruff check qiskit test tools examples setup.py qiskit_pkg + 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