Skip to content

Make unit tests pass as root and without a container runtime - #6875

Merged
rdimitrov merged 1 commit into
mainfrom
rdimitrov/env-robust-tests
Oct 20, 2026
Merged

rdimitrov merged 1 commit into
mainfrom
rdimitrov/env-robust-tests

Conversation

@rdimitrov

Copy link
Copy Markdown
Member

Summary

Running the unit suite as root, in a container with no container runtime and no keyring (the setup used by Anthropic's OSS Scanner, and by anyone running tests in a dev container), fails six packages. None of these failures is a code defect: each test assumes something about its environment. This PR makes those tests skip with a clear reason, or removes the assumption, without weakening what they assert when the precondition holds.

  • chmod-based write failures: root bypasses file permissions, so tests that force a write error by making a directory read-only cannot fail that write. A new osperm.SkipIfRoot(t) helper (test/testkit/osperm) skips them as root. It is used in pkg/client, pkg/plugins/pluginsvc, pkg/skills/skillsvc, and replaces the existing inline check in pkg/secrets.
  • pkg/api server tests: Build() created a real container runtime and client manager (which creates its own runtime), so the tests failed with no available runtime found. A newTestServerBuilder helper now injects gomock runtime, client, skill and plugin managers, so these tests run everywhere instead of being skipped.
  • Keyring: TestCompositeProvider_RealProviders skips when no keyring provider is available, instead of sniffing CI environment variables.
  • TestRunLLMSetup_PartialFailure: made claude-code's apply fail via a read-only .claude directory, which root writes through. It now uses a malformed settings.json, which fails for any user on any OS, so the Windows skip is gone too. Detection in this test never consults $PATH (the test entries set no binary name), so it does not depend on what is installed on the host.

Type of change

  • Bug fix
  • New feature
  • Refactoring (no behavior change)
  • Dependency update
  • Documentation
  • Other (describe): Test-only robustness fix

Test plan

  • Unit tests (task test)
  • E2E tests (task test-e2e)
  • Linting (task lint-fix)
  • Manual testing (describe below)

task test passes locally on macOS. Lint was run with the CI-pinned golangci-lint v2.15.0 on the touched packages (0 issues). The full unit suite was run in the OSS Scanner image (golang:1.27, as root) with docker run --rm --network none --cpus 2 --memory 8g; every package passes, and the tests above report SKIP with their reason where the precondition is missing.

Special notes for reviewers

Other chmod-based tests exist in the repo but already pass as root, so they were left unchanged.

🤖 Generated with Claude Code

Running the unit suite as root in a container with no container
runtime or keyring (as Anthropic's OSS Scanner does) failed six
packages for environmental reasons rather than code defects.

- Tests that force a write failure with chmod now skip as root, which
  bypasses permissions, via a new shared osperm.SkipIfRoot helper.
- pkg/api server tests inject mock runtime and client managers, so
  Build() no longer needs a container runtime on the host.
- The real-keyring test skips when no keyring provider is available
  instead of sniffing CI environment variables.
- The LLM partial-failure test breaks apply with malformed settings
  JSON instead of a read-only directory, so it runs as root and on
  Windows.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added the size/S Small PR: 100-299 lines changed label Oct 20, 2026
rdimitrov added a commit that referenced this pull request Oct 20, 2026
The environment-dependent unit tests now skip or use mocks (#6875), so
the suite passes in the scanner image and the expected-failure list is
no longer accurate.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rdimitrov rdimitrov mentioned this pull request Oct 20, 2026
2 of 7 tasks
@codecov

codecov Bot commented Oct 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 79.65%. Comparing base (89b1bd2) to head (e864fd9).

Files with missing lines Patch % Lines
test/testkit/osperm/osperm.go 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6875      +/-   ##
==========================================
- Coverage   79.77%   79.65%   -0.13%     
==========================================
  Files         820      821       +1     
  Lines       83723    83726       +3     
==========================================
- Hits        66791    66691     -100     
- Misses      16927    17030     +103     
  Partials        5        5              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

rdimitrov added a commit that referenced this pull request Oct 20, 2026
* Add OSS Scanner audit configuration

Register ToolHive with Anthropic's OSS Scanner. The Dockerfile gives
the scanner a reproducible build that runs the unit tests, and the
threat model tells it which inputs are untrusted and which failures
count as vulnerabilities.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>

* Drop expected test failures from scanner docs

The environment-dependent unit tests now skip or use mocks (#6875), so
the suite passes in the scanner image and the expected-failure list is
no longer accurate.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
@rdimitrov
rdimitrov merged commit 6354164 into main Oct 20, 2026
48 of 49 checks passed
@rdimitrov
rdimitrov deleted the rdimitrov/env-robust-tests branch October 20, 2026 08:21
@github-actions github-actions Bot mentioned this pull request Oct 22, 2026
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/S Small PR: 100-299 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant