Skip to content

ci: add per-component docker startup integration tests - #63

Open
mdanish98 wants to merge 1 commit into
mainfrom
feat/62__integ-tests
Open

ci: add per-component docker startup integration tests#63
mdanish98 wants to merge 1 commit into
mainfrom
feat/62__integ-tests

Conversation

@mdanish98

Copy link
Copy Markdown
Member

Closes #62

Summary

Adds a new integration-test CI job that boots the built Docker image for every AAS component (aas-repo, submodel-repo, aas-registry, submodel-registry) in an isolated matrix leg, gating merges on all four starting successfully. Before this change, only aas-repo was smoke-tested in CI — the other three components could silently regress until a downstream user hit the failure.

What changes

New CI job: integration-test

  • Runs after docker builds aas-mcp-server:test, before test-summary.
  • 4-way matrix (strategy.fail-fast: false) over the AAS components — a broken component fails only its own leg.
  • Rebuilds the image via cache-from: type=gha (near-instant cache hit off the warmed docker job).
  • test-summary now gates on integration-test in addition to test, lint, docker.
  • The existing single-component smoke test in the docker job is kept as a fast-fail signal for the image build itself; an inline comment notes that full per-component coverage lives in integration-test.

New helper: scripts/ci/docker-startup-check.sh

Two-part startup check that CI invokes per matrix leg and any developer can run locally:

  1. MCP handshake — pipes a single initialize JSON-RPC request into docker run -i, asserts .result.serverInfo.name == "AAS MCP Server (<component>)" via jq -e. Prints docker logs on failure.
  2. Uptime — starts a detached container with stdin held open via -i, sleeps UPTIME_SECONDS (default 10 s), asserts State.Running == true and State.ExitCode == 0. trap-based cleanup guarantees no leaked containers.

Env vars: UPTIME_SECONDS (default 10), IMAGE_TAG (default aas-mcp-server:test). --help documents local usage.

New integration fixtures: tests/fixtures/integration/

Five files, isolated from unit-test fixtures so they can evolve independently:

  • aas-repo-official-spec.yaml — hand-authored in the shape of tests/fixtures/sample_official_spec.yaml.
  • submodel-repo-official-spec.yaml, aas-registry-official-spec.yaml, submodel-registry-official-spec.yaml — each derived by manual chunking from the corresponding official IDTA spec (aas-specs) into a small, valid standalone OpenAPI 3.0 doc (≤200 lines, ≥1 GET, ≥1 path parameter, original operationIds preserved).
  • config.yaml.template — one config declaring all four components with curation.allowlist: [[get, "*"]].
  • README.md — documents the chunking rule and local usage.

Docs

  • tests/README.md — new "Docker Startup Integration Test (Local)" section with a one-liner developer usage pattern.

Design decisions (why this shape)

  • Matrix on a downstream job, not on docker. Keeps the image build as a single canonical step; each matrix leg pays only the cache-hit cost. Alternatives (bash loop, matrix on docker itself) rejected — see commit body for rationale.
  • Two-part check, not just handshake. Catches late-binding startup failures a single-shot handshake would miss (e.g. deferred spec-loading crashes after initialize).
  • Fixtures are derived, not vendored. Real official IDTA specs are up to ~143 KB / ~3900 lines — too large to be a signal in a Docker startup smoke test. Manual chunking keeps fixtures small (~50–100 lines) and readable; the source specs stay in the aas-specs repo.
  • Placeholder AAS_BASE_URL. The server does not dial the backend at startup; nothing needs to listen on that port. If future changes start eagerly probing the backend the integration test will fail loudly — that's the correct signal.
  • Whole-directory spec mount. The config validator eagerly validates every component's official_spec on load, so mounting only the selected component's spec fails validation for the other three. The script mounts the whole tests/fixtures/integration/ dir at /app/spec/; an inline comment documents why.

Verification

Local (docker build -t aas-mcp-server:test . + Docker Desktop 28.3.3):

PASS: aas-repo
PASS: submodel-repo
PASS: aas-registry
PASS: submodel-registry

Both the handshake and 10-second uptime sub-checks pass for every component against the actually-built image.

  • uv run pytest tests/ -m "not integration"259 passed, 13 skipped (matches baseline; no unit-test regression).
  • uv run reuse lint73/73 files compliant; new script and config template carry explicit SPDX headers, YAMLs covered by the aggregate path = "**" rule.
  • Fixtures validated: all four parse via yaml.safe_load, satisfy the structural check (openapi, info.title, info.version, non-empty paths, ≥1 GET, ≥1 path parameter), each ≤ 200 lines (81/67/67/67).

Compatibility

  • No changes to application source, Dockerfile, unit tests, production configs, release workflow.
  • CI runtime: +~1–2 minutes per PR (four short container boots run in parallel via matrix, warm cache).
  • No new runtime dependencies. Uses docker, jq, bash — all already on ubuntu-latest.
  • Reversible: revert this PR; nothing to un-migrate.

Non-goals

  • No real AAS backend or HTTP mock — startup check only, tool-call testing deferred.
  • No MCP tool-call testing beyond initialize.
  • No new pytest tests — this is a CI-level integration probe (shell + docker), not a code-level test.

Follow-ups for reviewers

  1. Please try one deliberate spec break locally (e.g. corrupt one fixture) to confirm the failing leg is isolated and test-summary correctly fails — I did not do this in a pushed commit to keep history clean.
  2. If the whole-directory mount strategy is uncomfortable, an alternative is to synthesize a component-specific single-file config at check time; happy to switch on request.

Adds a new 'integration-test' CI job that boots the built Docker image
for every AAS component (aas-repo, submodel-repo, aas-registry,
submodel-registry) in an isolated matrix leg, gating merges on all four
starting successfully.

Each leg runs scripts/ci/docker-startup-check.sh, which performs:
  1. an MCP 'initialize' handshake and asserts
     result.serverInfo.name == 'AAS MCP Server (<component>)';
  2. an uptime check — starts a detached container and verifies it is
     still running with exit code 0 after UPTIME_SECONDS (default 10).

The script is reusable locally against any freshly built
aas-mcp-server:test image.

Fixtures under tests/fixtures/integration/ are minimal, standalone
OpenAPI 3.0 documents (<=200 lines each): the aas-repo fixture is
hand-authored in the style of tests/fixtures/sample_official_spec.yaml;
the other three are derived by manual chunking from the official IDTA
specs (see aas-specs repo). Fixtures are isolated from unit-test
fixtures so changes to one do not force changes to the other.

The existing single-component smoke test in the 'docker' job is kept as
a fast-fail signal for the image build itself; a comment notes that
full four-component coverage now lives in 'integration-test'.

Local verification: all four components boot cleanly and pass the
two-part check against the actually-built image; existing pytest suite
unchanged (259 passed, 13 skipped, matches baseline).

Closes #62

@ricogu ricogu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice addition — per-component startup coverage was a real gap and the matrix shape is clean. One blocking item before merge: the Phase-1 trap can mask a failed handshake as a green run (inline on the script). The rest are should-fix robustness (word-splitting, mktemp) plus an optional tools/list assertion so the gate proves tools actually loaded, and one doc nit.

For what it's worth, all four legs and test-summary are green today — but the trap bug means a real handshake failure might not have surfaced, so worth fixing before relying on it as a gate.

INIT_REQUEST='{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2025-03-26","capabilities":{},"clientInfo":{"name":"docker-startup-check","version":"1.0"}}}'

HANDSHAKE_LOG="$(mktemp)"
trap 'rm -f "$HANDSHAKE_LOG"; cleanup' EXIT INT TERM

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: this can turn a failed handshake into a green run. The trap runs rm -f "$HANDSHAKE_LOG" before cleanup, and cleanup() reads local rc=$? — so $? is rm's exit code (0), not the original exit 1 from lines 137/147. A broken handshake would exit 0 and pass the gate.

Capture the status first:

cleanup() { local rc="${1:-$?}"; ...; exit "$rc"; }
trap 'rc=$?; rm -f "$HANDSHAKE_LOG"; cleanup "$rc"' EXIT INT TERM

Comment on lines +78 to +79
MOUNT_CONFIG="-v ${CONFIG_PATH}:/app/config/config.yaml:ro"
MOUNT_SPEC="-v ${SPEC_DIR}:/app/spec:ro"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These strings get expanded unquoted into docker run further down, so they word-split on spaces. Fine on the CI runner, but the script is documented as locally runnable and will break for a checkout under a path with spaces. Arrays are safer:

MOUNT_ARGS=(
  -v "${CONFIG_PATH}:/app/config/config.yaml:ro"
  -v "${SPEC_DIR}:/app/spec:ro"
)
# docker run ... "${MOUNT_ARGS[@]}" "${ENV_ARGS[@]}" ...

)

# Temp file for uptime-phase container ID; cleaned up on any exit.
UPTIME_CIDFILE="$(mktemp -u)"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: mktemp -u hands back a name without creating the file (TOCTOU). Low risk in CI, but easy to avoid — use a private dir and put the cidfile inside it:

CID_DIR="$(mktemp -d)"
UPTIME_CIDFILE="$CID_DIR/container.cid"
# ...clean up the dir in cleanup()

fi

if ! echo "$HANDSHAKE_LINE" | jq -e --arg name "$EXPECTED_NAME" '.result.serverInfo.name == $name' >/dev/null; then
echo "ERROR: handshake response did not match expected serverInfo.name=\"$EXPECTED_NAME\"" >&2

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The handshake only checks serverInfo.name, which is derived from AAS_COMPONENT — a component with zero exposed tools (e.g. a mis-parsed allowlist that curates everything away) would still pass here. Consider following up with tools/list and asserting it's non-empty, so the gate actually proves the spec + curation loaded something:

echo "$TOOLS_RESPONSE" | jq -e '.result.tools | length > 0'

Comment thread tests/README.md
Environment variables:
- `UPTIME_SECONDS` — how long the uptime phase waits (default `10`)
- `IMAGE_TAG` — image tag to test (default `aas-mcp-server:test`)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this points at docs/*.yaml, but the fixture README says the derived specs come from the external admin-shell-io/aas-specs repo (and aas-repo is hand-authored). Worth pointing this line at tests/fixtures/integration/README.md so future refreshers look in the right place.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add integration tests

2 participants