Skip to content

test(openclaw): add a config-generation environment fixture - #8559

Open
JulienAu wants to merge 6 commits into
NVIDIA:mainfrom
JulienAu:test/openclaw-env-fixture
Open

test(openclaw): add a config-generation environment fixture#8559
JulienAu wants to merge 6 commits into
NVIDIA:mainfrom
JulienAu:test/openclaw-env-fixture

Conversation

@JulienAu

@JulienAu JulienAu commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Nine config-generation suites carried a byte-identical BASE_ENV literal, six duplicated the buildTestEnv composition, and four the fake-openclaw stub writer. This is sequential PR 4 of issue #8289: it moves the ordinary valid generation environment, the stub writer, and the environment composition into test/helpers/openclaw-env-fixture.ts and migrates the nine suites. Following the isolation follow-up in #8557, the fixture ships with its own regression test proving per-call object independence.

Related Issue

Refs #8289 (sequential PR 4 of 4; PRs 1-3 are #8537, #8546, #8556)

Changes

  • Add test/helpers/openclaw-env-fixture.ts: baseOpenClawGenerationEnv() returning a fresh copy of the ordinary valid environment per call, ensureFakeOpenClaw(dir), and buildOpenClawTestEnv(dir, baseEnv, overrides) composing PATH, base entries, overrides, and HOME without mutating its inputs.
  • Add test/helpers/openclaw-env-fixture.test.ts: regression coverage for per-call independence and for composition that leaves base entries and overrides untouched, in the spirit of the test(onboard): preserve runtime fixture isolation #8557 isolation fix.
  • Migrate the nine suites with the byte-identical environment (generate-openclaw-config family, generate-openclaw-tool-disclosure-config, agents-manifest-policy-conformance, generate-openclaw-config-plugin-entries). The four suites whose BASE_ENV carries scenario-specific values (reasoning-effort, security-audit, audit-suppressions-real, generate-hermes-config) keep their local variants per the acceptance criteria, and the messaging-plan wrap stays at its call site.
  • Ratchet the ci/test-file-size-budget.json entry for generate-openclaw-config.test.ts from 1,941 to its new 1,915-line size, as the size gate requires after shrinking a legacy file.
  • Scope note: the issue's canonical corporate-CA test asset is deliberately not part of this PR. The inline PEM blocks are per-file scenario material (no two files share one), so canonicalizing them would change tested certificate content; that decision is left to a maintainer and can follow as its own change if wanted.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Quality Gates

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification:
  • Tests not applicable — justification:
  • Docs updated for user-facing behavior changes
  • Docs not applicable — justification: test fixtures only; no user-facing surface or documented behavior changes.
  • Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
  • Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification:
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Documentation Writer Review

  • Documentation writer subagent reviewed the completed changes
  • Result: no-docs-needed
  • Evidence: Test-only consolidation of repeated OpenClaw config-generation environments and an exact internal test-size-budget ratchet; no user-facing behavior, CLI, configuration, architecture, or documentation route changes.
  • Agent: Codex Desktop

DGX Station Hardware Evidence

  • Tested on DGX Station
  • Tested commit:
  • Station profile/scenario:
  • Result:
  • Supporting evidence:

Verification

  • PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub
  • Normal pre-commit, commit-msg, and pre-push hooks passed, and npm run check:diff passed after the current-main refresh.
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result: after the current main refresh, npx vitest run --project integration on the nine migrated suites plus the new fixture test passed (10 files, 178 tests). npm run typecheck:cli, npm run test-size:check, the normal commit hooks, and pre-push checks passed.
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result:
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: JulienAu 16043912+JulienAu@users.noreply.github.com

Summary by CodeRabbit

  • Tests
    • Standardized test environment setup across configuration and policy tests.
    • Added coverage for isolated fixtures, environment overrides, temporary-directory handling, and executable setup.
    • Updated the recorded test file size budget to reflect the reduced test footprint.

@copy-pr-bot

copy-pr-bot Bot commented Aug 7, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: d398d78a-de90-4a3c-b971-718603debcf5

📥 Commits

Reviewing files that changed from the base of the PR and between 22bc0cf and 251d5a1.

📒 Files selected for processing (1)
  • ci/test-file-size-budget.json
🚧 Files skipped from review as they are similar to previous changes (1)
  • ci/test-file-size-budget.json

📝 Walkthrough

Walkthrough

The PR adds shared OpenClaw test-environment helpers, migrates generation tests to use them, adds fixture coverage, and updates the test file-size budget.

Changes

OpenClaw fixture refactor

Layer / File(s) Summary
Shared OpenClaw environment fixture
test/helpers/openclaw-env-fixture.ts, test/helpers/openclaw-env-fixture.test.ts
Adds base environment creation, fake openclaw provisioning, environment overrides, temporary-directory HOME and PATH setup, and fixture tests.
Generation test migration
test/agents-manifest-policy-conformance.test.ts, test/generate-openclaw-config*.test.ts, test/generate-openclaw-tool-disclosure-config.test.ts, ci/test-file-size-budget.json
Migrates generation tests from local setup to shared helpers and reduces the recorded line budget from 1941 to 1915.

Estimated code review effort: 2 (Simple) | ~15 minutes

Possibly related PRs

Suggested labels: refactor, chore

Suggested reviewers: jyaunches

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 37.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the shared environment fixture added for OpenClaw config-generation tests.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

Nine config-generation suites carried a byte-identical BASE_ENV
literal, six duplicated the buildTestEnv composition, and four the
fake-openclaw stub writer. Move the ordinary valid generation
environment, the stub writer, and the env composition into
test/helpers/openclaw-env-fixture.ts, returning fresh objects per
call. The four suites whose BASE_ENV carries scenario-specific values
keep their local variants, and the messaging-plan wrap stays at its
call site. Every migrated file has a negative line delta, and the fixture carries
its own regression test proving per-call object independence.

Refs NVIDIA#8289

Signed-off-by: JulienAu <16043912+JulienAu@users.noreply.github.com>
@JulienAu
JulienAu force-pushed the test/openclaw-env-fixture branch from dd3e824 to f133ae5 Compare August 7, 2026 13:01
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — No blocking findings reported

Advisor assessment: No blocking advisor findings reported
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions

Model lanes

  • GPT-5.6 Terra (primary): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Failed

Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests.

2 semantic terminology decisions

Terminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.

  • justified — ordinary valid generation environment at test/helpers/openclaw-env-fixture.ts:9: Keep the term. The modifier distinguishes the shared valid baseline from scenario-specific overrides.
  • justified — fake OpenClaw at test/helpers/openclaw-env-fixture.ts:41: Keep the term. It identifies the test double and preserves the OpenClaw product-name form.

E2E guidance

Advisory only. A maintainer can dispatch the default E2E suite against this exact revision.

Recommended E2E: None

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

@apurvvkumaria apurvvkumaria self-assigned this Aug 7, 2026
Comment thread test/agents-manifest-policy-conformance.test.ts Fixed
Comment thread test/generate-openclaw-config-agents-manifest.test.ts Fixed
Comment thread test/generate-openclaw-config-gpt5-compat.test.ts Fixed
Comment thread test/generate-openclaw-config.test.ts Fixed
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
@apurvvkumaria

apurvvkumaria commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Maintainer follow-up on branch revision 251d5a180:

  • The earlier fixes remain intact: unused imports removed and both messaging-plan environment wrappers restored.
  • The approved branch is now refreshed onto current main. Its only conflict was the test-size budget: the resolution preserves this PR's reduced 1,915-line OpenClaw test budget and current main's 3,908-line installer-test budget.
  • Post-refresh validation passed: all 10 affected files / 178 tests, CLI type-check, test-size gate, diff check, commit hooks, and pre-push checks.
  • The refresh commit is signed, carries DCO sign-off, and appears as Verified on GitHub.
  • Documentation review remains no-docs-needed; the nine-category security review remains PASS with no findings because the diff is test-fixture consolidation with no runtime, credential, policy, or dependency change.
  • Fresh non-review gates are fully green on the current base: all eight CLI shards and aggregate checks, build/type-check/static/plugin/installer/audit lanes, CodeQL and ShellCheck, DCO, documentation receipt, commit lint, installer hash, growth guardrail, and CodeRabbit. The Advisor published merge_as_is with zero findings from Terra; the failed Nemotron lane is advisory.
  • The earlier independent approval predates this conflict-resolution commit, so the only remaining blocker is a renewed current-branch review. I will merge immediately after that review lands.

@jyaunches jyaunches self-assigned this Aug 7, 2026
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>

# Conflicts:
#	ci/test-file-size-budget.json
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants