[knoepfel]

Remove the inherited spack() function from the Spack setup test’s subprocess environment. This ensures spack is unavailable as intended, even when tests run from a Bash-initialized Spack environment, where restricting PATH alone is insufficient.

Aided by GPT-6.1 Sol

• Tests: The missing-command test now runs unset -f spack before sourcing setup definitions. This prevents an inherited Bash spack function from bypassing the test’s restricted PATH check.

[coderabbitai]

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 5cedf077-7e06-4ab5-8ec4-431c0adea07e

Commits

Reviewing files that changed from the base of the PR and between 9397fdd and 5d9fd8c.

Files selected for processing: 1

• scripts/test/test_devcontainer_host.py

Recent review details

Context from checks skipped due to timeout: 3

• GitHub Check: Analyze python with CodeQL
• GitHub Check: CodeRabbit
• GitHub Check: clang-tidy-check

Additional context used

Code guidelines: 1

.github/copilot-instructions.md — auto-discovered

Path-based instructions: 1

Source excerpt: Use ruff for formatting and linting (configured in `pyproject.toml`) Source excerpt: Follow Google-style docstring conventions Source excerpt: Line length: 99 characters Source excerpt: Use double quotes for strings Source e...

CodeRabbit inference engine (.github/copilot-instructions.md)

Files:

• scripts/test/test_devcontainer_host.py

Additional comments: 1

scripts/test/test_devcontainer_host.py: 1

795-796: LGTM!

Walkthrough

The test clears any inherited spack shell function before sourcing setup definitions and checking the missing-command path.

Changes: Spack command lookup test

Layer / File(s)	Summary
Clear inherited Spack function scripts/test/test_devcontainer_host.py	The test runs unset -f spack before sourcing setup definitions.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Priority: Low

Change: Other

Possibly related issues

• Framework-R-D/phlex#995: This issue concerns test coverage and runtime requirements for the pre-commit hook; this change isolates one test run by removing an inherited spack function.

Possibly related PRs

• Framework-R-D/phlex#975: This PR added the test and its missing-command coverage, which this change isolates from an inherited Bash function.

Suggested reviewers: greenc-fnal

Merge Risk: Minimal · previous review 10304

The focused test-environment change is ready to merge.

Pre-merge checks: 4

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: isolating the Spack setup test from an inherited Bash function.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

@@            Coverage Diff             @@
##             main    #1017      +/-   ##
==========================================
- Coverage   89.59%   88.87%   -0.73%     
==========================================
  Files         177      189      +12     
  Lines        5949     7712    +1763     
  Branches      975      975              
==========================================
+ Hits         5330     6854    +1524     
- Misses        392      630     +238     
- Partials      227      228       +1     

Flag	Coverage Δ
amd64	89.49% <ø> (-0.11%)	⬇️
arm64	88.87% <ø> (-0.73%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 13 files with indirect coverage changes

---

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9397fdd...5d9fd8c. Read the comment docs.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[greenc-FNAL]

This is OK as-is, but if the need for spack-clean init is inherent to the fixture, then the better-placed solution might be to put unset -f spack at the beginning of the fixture script.

[greenc-FNAL]

@coderabbitai summary

[coderabbitai]

✅ Action performed

Summary regenerated.