diff --git a/.github/scripts/run_notebook.py b/.github/scripts/run_notebook.py index 94b66ee..db15e00 100644 --- a/.github/scripts/run_notebook.py +++ b/.github/scripts/run_notebook.py @@ -69,11 +69,54 @@ def run(cmd, **kw): return subprocess.run(cmd, capture_output=True, text=True, **kw) +ANSI_RE = re.compile(r"\x1b\[[0-9;]*m") + + +def execute_with_kernel(tmp_nb: Path, executed_path: Path, nb_dir: Path, + timeout: int, log_path: Path, finalize) -> int: + """Run the notebook through a real kernel via nbconvert, keeping outputs. + + --allow-errors keeps going past a failing cell so the executed notebook + shows reviewers exactly where and how it failed; the error outputs are + then scanned to decide pass/fail. + """ + executed_path.parent.mkdir(parents=True, exist_ok=True) + cmd = ["jupyter", "nbconvert", "--to", "notebook", "--execute", "--allow-errors", + f"--ExecutePreprocessor.timeout={timeout}", + "--ExecutePreprocessor.kernel_name=python3", + "--output", str(executed_path.resolve()), str(tmp_nb.resolve())] + print(f"\n=== executing notebook through the kernel ===\n {' '.join(cmd)}", flush=True) + r = run(cmd, cwd=str(nb_dir)) + with log_path.open("a") as f: + f.write(f"\n=== nbconvert rc={r.returncode} ===\n{r.stderr[-4000:]}\n") + if r.returncode != 0 or not executed_path.exists(): + return finalize("execute", False, error=r.stderr[-3000:]) + + executed = nbformat.read(str(executed_path), as_version=4) + for i, cell in enumerate(executed.cells): + if cell.cell_type != "code": + continue + for out in cell.get("outputs", []): + if out.get("output_type") == "error": + tb = ANSI_RE.sub("", "\n".join(out.get("traceback", []))) + print(tb, flush=True) + return finalize("execute", False, + error=f"cell {i}: {out.get('ename')}: {out.get('evalue')}\n{tb}", + executed_notebook=str(executed_path)) + return finalize("done", True, executed_notebook=str(executed_path)) + + def main() -> int: parser = argparse.ArgumentParser() parser.add_argument("notebook") parser.add_argument("--output-dir", default="/tmp/notebook-test") parser.add_argument("--timeout", type=int, default=3600) + parser.add_argument( + "--executed-output", + help="Execute through the real Jupyter kernel (nbconvert) instead of the " + "ipython script path and write the executed notebook, with outputs, " + "to this path. Used by the PR workflow to publish a reviewable copy.", + ) args = parser.parse_args() nb_path = Path(args.notebook) @@ -109,10 +152,10 @@ def finalize(stage: str, ok: bool, error: str | None = None, **extra) -> int: log_path.write_text("") - r = run( - ["uv", "pip", "install", "--system", - "ipython", "nbconvert", "nbformat", *pins] - ) + harness_deps = ["ipython", "nbconvert", "nbformat"] + if args.executed_output: + harness_deps.append("ipykernel") + r = run(["uv", "pip", "install", "--system", *harness_deps, *pins]) with log_path.open("a") as f: f.write(f"=== install rc={r.returncode} ===\n") f.write(f"--- stdout ---\n{r.stdout[-2000:]}\n") @@ -142,6 +185,10 @@ def finalize(stage: str, ok: bool, error: str | None = None, **extra) -> int: tmp_nb = out_dir / f"{slug}.toexec.ipynb" nbformat.write(nb_for_exec, str(tmp_nb)) + if args.executed_output: + return execute_with_kernel(tmp_nb, Path(args.executed_output), nb_dir, + args.timeout, log_path, finalize) + r = run(["jupyter", "nbconvert", "--to", "script", "--stdout", str(tmp_nb)]) if r.returncode != 0: return finalize("convert", False, error=r.stderr[-3000:]) diff --git a/.github/workflows/index_workflow.yml b/.github/workflows/index_workflow.yml index d35c56d..ea1e20f 100644 --- a/.github/workflows/index_workflow.yml +++ b/.github/workflows/index_workflow.yml @@ -14,7 +14,7 @@ on: # Deploys push to the same gh-pages branch; never run two at once. concurrency: - group: index-deploy + group: gh-pages-deploy cancel-in-progress: false jobs: diff --git a/.github/workflows/pr_preview.yml b/.github/workflows/pr_preview.yml index e20fcca..19e5ba3 100644 --- a/.github/workflows/pr_preview.yml +++ b/.github/workflows/pr_preview.yml @@ -15,6 +15,11 @@ jobs: permissions: contents: write pull-requests: write + # Every gh-pages deploy across workflows shares this group so pushes to + # the branch never race. + concurrency: + group: gh-pages-deploy + cancel-in-progress: false steps: - uses: actions/checkout@v4 @@ -61,6 +66,9 @@ jobs: permissions: contents: write pull-requests: write + concurrency: + group: gh-pages-deploy + cancel-in-progress: false steps: - name: Checkout gh-pages uses: actions/checkout@v4 diff --git a/.github/workflows/test-changed-notebooks.yml b/.github/workflows/test-changed-notebooks.yml index c52cadb..6eade39 100644 --- a/.github/workflows/test-changed-notebooks.yml +++ b/.github/workflows/test-changed-notebooks.yml @@ -67,11 +67,38 @@ jobs: run: pip install --no-cache-dir uv nbformat - name: Run notebook + id: run + # Executes through the real kernel and keeps the executed notebook so + # reviewers can see the outputs. run: | + slug=$(python -c "import sys; sys.path.insert(0, '.github/scripts'); from run_notebook import slugify; print(slugify(sys.argv[1]))" "${{ matrix.notebook }}") + echo "slug=$slug" >> "$GITHUB_OUTPUT" python .github/scripts/run_notebook.py \ "${{ matrix.notebook }}" \ --output-dir "$RUNNER_TEMP/notebook-test" \ - --timeout 3600 + --timeout 3600 \ + --executed-output "$RUNNER_TEMP/executed/$slug.ipynb" + + - name: Render executed notebook to HTML + if: always() && steps.run.outputs.slug != '' + run: | + slug='${{ steps.run.outputs.slug }}' + if [ -f "$RUNNER_TEMP/executed/$slug.ipynb" ]; then + jupyter nbconvert --to html "$RUNNER_TEMP/executed/$slug.ipynb" \ + --output "$slug.html" --output-dir "$RUNNER_TEMP/executed" + if [ '${{ job.status }}' = 'success' ]; then status=pass; else status=fail; fi + printf '%s\t%s\t%s\n' "$slug" "${{ matrix.notebook }}" "$status" \ + > "$RUNNER_TEMP/executed/$slug.status.tsv" + fi + + - name: Upload executed notebook + if: always() && steps.run.outputs.slug != '' + uses: actions/upload-artifact@v4 + with: + name: executed-${{ steps.run.outputs.slug }} + path: ${{ runner.temp }}/executed/ + retention-days: 14 + if-no-files-found: ignore - name: Upload artifacts on failure if: failure() @@ -80,3 +107,66 @@ jobs: name: failure-${{ strategy.job-index }} path: ${{ runner.temp }}/notebook-test/ retention-days: 14 + + publish-executed: + name: Publish executed notebooks to the PR preview + needs: [detect, test] + # Fork PRs only get a read-only token, so they keep the artifacts but + # cannot deploy to gh-pages; same-repo PRs get clickable previews. + if: always() && needs.detect.outputs.any_changed == 'true' && github.event.pull_request.head.repo.full_name == github.repository + runs-on: ubuntu-latest + permissions: + contents: write + pull-requests: write + concurrency: + group: gh-pages-deploy + cancel-in-progress: false + steps: + - name: Download executed notebooks + uses: actions/download-artifact@v4 + with: + pattern: executed-* + path: executed + merge-multiple: true + + - name: Assemble preview files and comment + id: assemble + run: | + mkdir -p publish + cp executed/*.html publish/ 2>/dev/null || true + base="https://notebooks.dandiarchive.org/pr-preview/${{ github.event.pull_request.number }}/notebooks" + { + echo "## Executed notebooks" + echo + echo "CI executed the notebooks changed in this PR through a Jupyter kernel. Review the outputs here (each link is the notebook with its outputs, as a reader would see it):" + echo + for f in executed/*.status.tsv; do + [ -f "$f" ] || continue + IFS=$'\t' read -r slug notebook status < "$f" + if [ "$status" = pass ]; then mark="✅"; else mark="❌"; fi + echo "- $mark [\`$notebook\`]($base/$slug.html)" + done + echo + echo "_Last updated: commit \`${{ github.event.pull_request.head.sha }}\`. The executed copies live only in the preview and are removed when the PR closes; nothing is committed to the branch._" + } > comment.md + cat comment.md + if ls publish/*.html >/dev/null 2>&1; then echo "has_files=true" >> "$GITHUB_OUTPUT"; else echo "has_files=false" >> "$GITHUB_OUTPUT"; fi + + - name: Deploy executed notebooks to gh-pages + if: steps.assemble.outputs.has_files == 'true' + uses: peaceiris/actions-gh-pages@v3 + with: + github_token: ${{ secrets.GITHUB_TOKEN }} + publish_dir: ./publish + destination_dir: pr-preview/${{ github.event.pull_request.number }}/notebooks + keep_files: true + commit_message: "Deploy executed notebooks for PR #${{ github.event.pull_request.number }}" + user_name: 'github-actions[bot]' + user_email: 'github-actions[bot]@users.noreply.github.com' + + - name: Comment PR with executed notebook links + if: steps.assemble.outputs.has_files == 'true' + uses: marocchino/sticky-pull-request-comment@v2 + with: + header: executed-notebooks + path: comment.md diff --git a/docs/adding-notebooks.md b/docs/adding-notebooks.md index 77d8178..b0b16a3 100644 --- a/docs/adding-notebooks.md +++ b/docs/adding-notebooks.md @@ -52,6 +52,14 @@ on an `ubuntu-latest` runner with Python 3.12. That script: > imports something the install cell doesn't pin, CI fails (even if it "works on > Colab," where that package happens to be preinstalled). +For pull requests, CI also executes each changed notebook through a real +Jupyter kernel and publishes the executed copy, outputs included, to the PR's +preview site, linking it from a comment on the PR so reviewers can read the +rendered results without running anything. The executed copies live only in +the preview and are removed when the PR closes; nothing is committed to the +branch. (PRs from forks get the executed notebooks as workflow artifacts +instead, since fork workflows cannot deploy the preview.) + `test-changed-notebooks.yml` only tests notebooks **changed in the PR**. A notebook already on `master` that would fail today is not re-run until it is touched again (or until the weekly sweep catches it). Don't assume "it's on