Skip to content

test(docker): align Dockerfile contract tests with simplified TUI flow - #19024

Closed
Sanjays2402 wants to merge 1 commit into
NousResearch:mainfrom
Sanjays2402:fix/main-ci-dockerfile-tui-test-asserts
Closed

test(docker): align Dockerfile contract tests with simplified TUI flow#19024
Sanjays2402 wants to merge 1 commit into
NousResearch:mainfrom
Sanjays2402:fix/main-ci-dockerfile-tui-test-asserts

Conversation

@Sanjays2402

Copy link
Copy Markdown
Contributor

Summary

Fixes two Tests failures observed on main (and therefore propagating to every open PR):

FAILED tests/tools/test_dockerfile_pid1_reaping.py::test_dockerfile_installs_tui_dependencies
FAILED tests/tools/test_dockerfile_pid1_reaping.py::test_dockerfile_materializes_local_tui_ink_package

Reference run: 25250051126 on 5d3be898a.

Root cause

The Dockerfile dropped the manual @hermes/ink materialisation gymnastics in favour of letting npm workspaces resolve the bundled package naturally. Two contract tests still asserted the older flow.

test_dockerfile_installs_tui_dependencies required:

assert "ui-tui/packages/hermes-ink/package-lock.json" in dockerfile_text

…but the lockfile is no longer COPY'd individually — the entire ui-tui/packages/hermes-ink/ tree is COPY'd instead (the workspace reference from ui-tui/package.json is file: so npm needs the real source, not just a manifest stub).

test_dockerfile_materializes_local_tui_ink_package required a 7-clause conjunction matching very specific rm -rf / npm install --omit=dev / --prefix node_modules/@hermes/ink / rm -rf .../react invocations that were stripped out when the workspace resolution was simplified.

Fix

Pin the contract the image has to satisfy (zombie reaping + bundled workspace package resolves) rather than the exact shell incantations the old flow used:

  • TUI deps install: ui-tui/package.json + ui-tui/package-lock.json + ui-tui/packages/hermes-ink/ tree are all COPY'd, and an npm install/ci step runs in ui-tui.
  • Bundled hermes-ink: the workspace package source is COPY'd (so await import('@hermes/ink') resolves at runtime).

Keeps the spirit of #15012 (zombie reaping) / #16690 (bundled workspace materialisation) without locking the Dockerfile into one specific implementation flavour.

Validation

$ pytest tests/tools/test_dockerfile_pid1_reaping.py -q
6 passed in 1.43s

Scope

  • ✅ No production code change (test-only)
  • ✅ All 6 dockerfile contract tests pass
  • ✅ The pid1/tini contract (the actual reason the file exists) is unchanged

Out of scope

The other ~9 main-CI failures — separate focused PRs (#18972, #18974, #18977, #18979 already up; #18980 dotenv inbound; this one is the next).

The Dockerfile dropped the manual `@hermes/ink` materialisation gymnastics
in favour of letting npm workspaces resolve the bundled package
naturally. Two contract tests still asserted the older flow:

`test_dockerfile_installs_tui_dependencies` required:
    'ui-tui/packages/hermes-ink/package-lock.json' in dockerfile_text

…but the lockfile is no longer COPIED individually \u2014 the entire
`ui-tui/packages/hermes-ink/` tree is COPIED instead (the workspace
reference from `ui-tui/package.json` is `file:` so npm needs the
real source, not just a manifest stub).

`test_dockerfile_materializes_local_tui_ink_package` required a 7-clause
conjunction matching specific `rm -rf` / `npm install --omit=dev`
`--prefix node_modules/@hermes/ink` / `rm -rf .../react` invocations
that were stripped out when the workspace resolution was simplified.

Update the assertions to pin the *contract* the image actually has to
carry rather than the *exact shell incantations* the old flow used:

* TUI deps install: ui-tui/package.json + ui-tui/package-lock.json +
  ui-tui/packages/hermes-ink/ tree are all COPIED, and an npm
  install/ci step runs in ui-tui.
* Bundled hermes-ink: the workspace package source is COPIED (so
  `await import('@hermes/ink')` resolves at runtime).

This keeps the spirit of NousResearch#15012 / NousResearch#16690 (zombie reaping + bundled
workspace materialisation must continue to work) without locking the
Dockerfile into one specific implementation flavour.

Validation:

    $ pytest tests/tools/test_dockerfile_pid1_reaping.py -q
    6 passed in 1.43s

No production code change. Fixes the two failures observed on `main`
(run 25250051126):

`tests/tools/test_dockerfile_pid1_reaping.py::test_dockerfile_installs_tui_dependencies`
`tests/tools/test_dockerfile_pid1_reaping.py::test_dockerfile_materializes_local_tui_ink_package`
@alt-glitch alt-glitch added type/test Test coverage or test infrastructure P3 Low — cosmetic, nice to have area/docker Docker image, Compose, packaging comp/tui Terminal UI (ui-tui/ + tui_gateway/) labels May 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/docker Docker image, Compose, packaging comp/tui Terminal UI (ui-tui/ + tui_gateway/) P3 Low — cosmetic, nice to have type/test Test coverage or test infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants