Skip to content

No CI coverage for .devcontainer/** and scripts/** — silent provisioning failures can ship #98

Description

@ayeshurun

Problem

.github/workflows/fab-build.yml path-filters its triggers to:

paths:
  - "src/**"
  - "tests/**"
  - "pyproject.toml"
  - "tox.toml"
  - "requirements*.txt"
  - ".github/workflows/fab-build.yml"

As a result, changes to .devcontainer/**, scripts/**, and .gitignore receive zero CI coverage.

This is not theoretical. Verified on PR #97, which changes .devcontainer/devcontainer.json, scripts/install_dev_container_dependencies.sh, .devcontainer/local.env.example and .gitignore:

$ gh pr checks 97
Check Pull Request Title   pending
🔄 Check Changelog          pending
⏭️  Skip Changelog           skipping

fab-build does not appear at all.

Why it matters

PR #97 repaired a bug that had been live in scripts/install_dev_container_dependencies.sh: the script ran

apt-get update && apt-get install -y ...

POSIX ignores errexit for any command in an AND-OR list other than the last, so a failing apt-get update short-circuited the install, and the script exited 0 with cmake, pkg-config, libcairo2-dev and python3-dev never installed. Contributors got a container that looked healthy and failed later at build time.

The absence of CI on this path is a plausible reason that went unnoticed. Two independent reviewers on PR #97 flagged the gap.

Proposed work

Add a workflow (or a job in an existing one) triggered on:

  • scripts/**
  • .devcontainer/**
  • .gitignore
  • the workflow file itself

It should:

  1. Run bash -n on repository shell scripts.
  2. Run ShellCheck. scripts/install_dev_container_dependencies.sh is currently clean at --severity=style, so this can start strict without a backlog.
  3. Run a smoke test of scripts/install_dev_container_dependencies.sh in mcr.microsoft.com/devcontainers/python:1-3.12-bullseye as the non-root vscode user, on linux/amd64.

Critical requirement for the smoke test

Assert observable outcomes, not the script's exit code. An exit-code-only assertion is precisely what hid the original bug — the script returned 0 while installing nothing. The test must assert at minimum:

  • each of cmake, pkg-config, libcairo2-dev, python3-dev is actually present (dpkg -s)
  • /etc/apt/sources.list.d/yarn.list has been removed
  • changie --version prints the expected version (note: changie version is not a subcommand and will produce a false failure)

Open questions

  • arm64 coverage. Many contributors develop on Apple Silicon, and the script has architecture-dependent logic mapping uname -m to release asset names. Worth covering if the runner strategy permits, possibly via QEMU.
  • Network dependence. The script reaches github.com, deb.debian.org and PyPI. GitHub-hosted runners reach all three, but this makes the job sensitive to upstream availability. Consider whether the pip step should be skippable in CI to keep the job from flaking on PyPI incidents.
  • Whether this belongs as a new workflow or an additional job in fab-build.yml.

Context

Split out of PR #97 deliberately, to keep a network-unblocking bugfix from growing a CI surface. Not urgent, but it is the reason a silent provisioning failure shipped.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions