Skip to content

Commit

Permalink
Separate repo-specific functionality from nb-tester (#2425)
Browse files Browse the repository at this point in the history
Splits out part of #2405

This PR pulls some of our CI functionality out of the nb-tester package
and into separate scripts.
  • Loading branch information
frankharkins authored Dec 5, 2024
1 parent 5aaf222 commit 1005805
Show file tree
Hide file tree
Showing 7 changed files with 128 additions and 86 deletions.
6 changes: 4 additions & 2 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,10 @@ jobs:
- name: Setup Python environment
uses: ./.github/actions/set-up-notebook-testing
- name: nb-tester tests
run: tox -e nb-tester

- name: Check all notebooks are listed in the config file
run: python scripts/ci/check-all-notebooks-are-tested.py

- name: Lint notebooks
shell: python
run: |
Expand Down
58 changes: 27 additions & 31 deletions .github/workflows/notebook-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,52 +25,48 @@ jobs:
steps:
- uses: actions/checkout@v4

- name: Get all changed notebooks
id: all-changed-notebooks
- name: Get relevant changed files
id: all-changed
uses: tj-actions/changed-files@af2816c65436325c50621100d67f6e853cd1b0f1
with:
files: "docs/**/*.ipynb"
files: "{docs/**/*.ipynb,scripts/nb-tester/**/*}"
separator: "\n"
- name: Get changed config files
id: changed-config
uses: tj-actions/changed-files@af2816c65436325c50621100d67f6e853cd1b0f1
with:
files: "scripts/nb-tester/**/*"
- name: Check for notebooks that require Linux
id: linux-changed-files
uses: tj-actions/changed-files@af2816c65436325c50621100d67f6e853cd1b0f1
with:
write_output_files: true

- name: Check if extra linux deps needed
id: check-deps
shell: python
run: |
# Add your notebook to this list if it needs latex or graphviz to run
files: |
docs/guides/visualize-circuits.ipynb
docs/guides/custom-backend.ipynb
docs/guides/transpiler-stages.ipynb
docs/guides/represent-quantum-computers.ipynb
docs/guides/common-parameters.ipynb
EXTRA_DEPS_NOTEBOOKS = """\
docs/guides/visualize-circuits.ipynb
docs/guides/custom-backend.ipynb
docs/guides/transpiler-stages.ipynb
docs/guides/represent-quantum-computers.ipynb
docs/guides/common-parameters.ipynb
""".strip().split("\n")
import os
github_output = os.getenv("GITHUB_OUTPUT")
all_files = "${{ steps.all-changed.outputs.all_changed_files }}".split("\n")
config_changed = any(path.startswith("scripts/") for path in all_files)
extra_deps = config_changed or any(path in EXTRA_DEPS_NOTEBOOKS for path in all_files)
with open(github_output, "a") as output:
output.write(f"NEEDS_EXTRA_DEPS={str(extra_deps).lower()}")
- name: Setup environment
uses: ./.github/actions/set-up-notebook-testing
with:
# Install Linux deps if the specific guides were changed, or
# if all files are being tested.
install-linux-deps: ${{ steps.linux-changed-files.outputs.any_changed == 'true' || steps.changed-config.outputs.any_changed == 'true' }}
install-linux-deps: ${{ steps.check-deps.outputs.NEEDS_EXTRA_DEPS }}
ibm-quantum-token: ${{ secrets.IBM_QUANTUM_TEST_TOKEN }}

- name: Execute notebooks
shell: python
env:
PR_REPOSITORY: ${{ github.event.pull_request.head.repo.full_name }}
run: |
import os
import subprocess
changed_notebooks = """${{ steps.all-changed-notebooks.outputs.all_changed_files }}"""
changed_config = """${{ steps.changed-config.outputs.all_changed_files }}"""
args = ["tox", "--", "--write"]
if changed_notebooks and not changed_config:
args.extend(changed_notebooks.split("\n"))
if os.environ["PR_REPOSITORY"] != os.environ["GITHUB_REPOSITORY"]:
args.append("--is-fork")
subprocess.run(args, check=True)
run: python scripts/ci/pr-execute-notebooks.py

- name: Detect changed notebooks
id: changed-notebooks
Expand Down
42 changes: 42 additions & 0 deletions scripts/ci/check-all-notebooks-are-tested.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# This code is a Qiskit project.
#
# (C) Copyright IBM 2024.
#
# This code is licensed under the Apache License, Version 2.0. You may
# obtain a copy of this license in the LICENSE file in the root directory
# of this source tree or at http://www.apache.org/licenses/LICENSE-2.0.
#
# Any modifications or derivative works of this code must retain this
# copyright notice, and modified files need to carry a notice indicating
# that they have been altered from the originals.

"""
Check all notebooks are listed in `scripts/config/notebook-testing.toml` so
they don't slip through our CI tests.
"""

from pathlib import Path
import tomllib
import sys

config_path = Path("scripts/config/notebook-testing.toml")
config = tomllib.loads(config_path.read_text())

categorized_notebooks = set()
for group in config.values():
for path in group:
categorized_notebooks.add(Path(path))

uncategorized = [
path
for path in Path(".").glob("[!.]*/**/*.ipynb")
if not path.match("**/.ipynb_checkpoints/**") and path not in categorized_notebooks
]

if uncategorized:
print(
f"\nThe following notebooks are not classified in {config_path}:\n "
+ "\n ".join(map(str, uncategorized))
+ "\nAdd them to the appropriate group so we know how to test them.\n"
)
sys.exit(1)
55 changes: 55 additions & 0 deletions scripts/ci/pr-execute-notebooks.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
# This code is a Qiskit project.
#
# (C) Copyright IBM 2024.
#
# This code is licensed under the Apache License, Version 2.0. You may
# obtain a copy of this license in the LICENSE file in the root directory
# of this source tree or at http://www.apache.org/licenses/LICENSE-2.0.
#
# Any modifications or derivative works of this code must retain this
# copyright notice, and modified files need to carry a notice indicating
# that they have been altered from the originals.

"""
Run the notebook tester on changed notebooks and display a helpful message if
can't test due to being on a fork.
"""

FORK_ERROR_MESSAGE = """\
We can't run this test on pull requests from forks.
If you have write access to Qiskit/documentation, push to a new branch there
and make your pull request from that branch instead.
If you don't have write access, you must test out the notebook locally using
the instructions in https://github.com/Qiskit/documentation#execute-notebooks.
When this PR is approved, a maintainer will merge it to a new branch in
Qiskit/documentation, then make a PR from that branch into main so it can pass
CI.
"""

import os
import subprocess
from pathlib import Path

all_changed_files = Path(".github/outputs/all_changed_files.txt").read_text().split("\n")

changed_notebooks = [
path for path in all_changed_files
if path.startswith("docs/")
]
config_changed = any(path.startswith("scripts/") for path in all_changed_files)

args = ["tox", "--", "--write"]
if changed_notebooks and not config_changed:
args.extend(changed_notebooks.split("\n"))

is_fork = os.environ["PR_REPOSITORY"] != os.environ["GITHUB_REPOSITORY"]

try:
subprocess.run(args, check=True)
except Exception as err:
is_fork = os.environ["PR_REPOSITORY"] != os.environ["GITHUB_REPOSITORY"]
if is_fork:
raise SystemExit(FORK_ERROR_MESSAGE) from err
raise err
1 change: 0 additions & 1 deletion scripts/config/notebook-testing.toml
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ notebooks_normal_test = [

# Don't test the following notebooks (this section can include glob patterns)
notebooks_exclude = [
"**/.ipynb_checkpoints/**",
"docs/guides/qedma-qesem.ipynb",
"docs/guides/q-ctrl-optimization-solver.ipynb",
"docs/guides/q-ctrl-performance-management.ipynb",
Expand Down
24 changes: 0 additions & 24 deletions scripts/nb-tester/qiskit_docs_notebook_tester/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,22 +164,6 @@ def notebooks_to_execute(self) -> Iterator[Path]:
)
continue

if self.args.is_fork:
print(
"⛔️ We can't run notebook tests on pull requests from forks "
"because of how GitHub Secrets work."
"\n\n"
"If you have write access to Qiskit/documentation, push to a new "
"branch there and make your pull request from that branch instead."
"\n\n"
"If you don't have write access, you should locally test out the notebooks you're modifying "
"by using the instructions in "
"https://github.com/Qiskit/documentation#execute-notebooks. "
"When this PR is approved, a maintainer will merge it to a new "
"branch in Qiskit/documentation, then make a PR from that branch "
"into main so it can pass CI.\n",
)
sys.exit(1)
yield path

def should_patch(self, path: Path) -> bool:
Expand Down Expand Up @@ -242,14 +226,6 @@ def get_args() -> argparse.Namespace:
"--config-path",
help="Path to a TOML file containing the globs for detecting and sorting notebooks",
)
parser.add_argument(
"--is-fork",
action="store_true",
help=(
"Set to true when running on forks in CI, since authentication cannot work there. "
"The program will fail with a helpful message if any of the notebooks cannot be executed."
),
)
parser.add_argument(
"--provider",
action="store",
Expand Down
28 changes: 0 additions & 28 deletions scripts/nb-tester/test/test_notebook_classification.py

This file was deleted.

0 comments on commit 1005805

Please sign in to comment.