Skip to content

fix(nv-boot): make notice_check_test work in the monorepo layout#324

Merged
balajinvda merged 1 commit into
mainfrom
fix/quarantine-notice-check
Jul 21, 2026
Merged

fix(nv-boot): make notice_check_test work in the monorepo layout#324
balajinvda merged 1 commit into
mainfrom
fix/quarantine-notice-check

Conversation

@balajinvda

@balajinvda balajinvda commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Why

The #269 merge turned main red once #311 enabled root tests: //src/libraries/java/nv-boot-parent/tools/bazel:notice_check_test failed with No module named 'generate_notice'. It was written for nv-boot-parent as a standalone workspace root, so it resolved its helper/metadata at ${workspace}/tools/bazel/... and checked ${workspace}/NOTICE — but in the monorepo the workspace root is the repo root (files live under src/libraries/java/nv-boot-parent/...), and ${workspace}/NOTICE is the aggregate root NOTICE (Go+Rust+Java) that a Java-only regeneration can never match.

What changed (proper fix, not a quarantine)

  • notice_check_test.sh: resolve generate_notice.py, metadata, and roots under the src/libraries/java/nv-boot-parent prefix, and check nv-boot-parent's own NOTICE. The Maven lock stays the single root //:maven_install.json.
  • tools/bazel/BUILD.bazel: swap the //:NOTICE data dep for //src/libraries/java/nv-boot-parent:NOTICE.

The test now validates nv-boot-parent's NOTICE (its Java closure) against the root lock, which is what it was designed to do. No quarantine.

Testing

Script passes bash -n; the root row runs the test (no longer skipped). Green root here confirms the NOTICE is fresh and the test passes.

Issues

Closes #323

References

Broke via #269 after #311 enabled root tests.

Dependencies

None

@balajinvda
balajinvda requested a review from a team as a code owner July 21, 2026 20:27
@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a Bazel negative-test quarantine entry for notice_check_test and documents CI failures involving Python module resolution and NOTICE-check scope mismatch.

Changes

Bazel test quarantine

Layer / File(s) Summary
Quarantine NOTICE check test
.github/bazel-root-test-quarantine.txt
Adds the notice_check_test negative pattern and documents its GitHub Actions failure causes and tracking reference.

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

Possibly related issues

Possibly related PRs

  • NVIDIA/nvcf#311 — Introduces the quarantine mechanism used by this update.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title names the right test and layout, but says it was fixed when the PR actually quarantines it instead. Rename it to reflect the quarantine, e.g. 'quarantine notice_check_test in the monorepo layout'.
✅ Passed checks (4 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/quarantine-notice-check

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/bazel-root-test-quarantine.txt:
- Around line 26-34: Update the header of the quarantine file so it no longer
claims all entries belong to nvcf-go or universally track NVIDIA/nvcf#284. Make
the header generic, or move ownership and tracking details into comments
associated with each entry while preserving the nv-boot entry’s NVIDIA/nvcf#323
metadata.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 4a2bbad3-b0e9-4e05-9514-e97e7730e16c

📥 Commits

Reviewing files that changed from the base of the PR and between 5e459d7 and f4d7ea0.

📒 Files selected for processing (1)
  • .github/bazel-root-test-quarantine.txt

Comment thread .github/bazel-root-test-quarantine.txt Outdated
The test was written for nv-boot-parent as a standalone workspace root, so it
resolved generate_notice.py and its metadata at ${workspace}/tools/bazel/... and
checked ${workspace}/NOTICE. In the monorepo the workspace root is the repo
root, so those files live under src/libraries/java/nv-boot-parent/tools/bazel and
the import failed (No module named 'generate_notice'); it also checked the
aggregate root NOTICE (Go+Rust+Java) with only nv-boot's Java roots, which can
never match.

Point the script at the subtree prefix and at nv-boot-parent's own NOTICE, and
swap the //:NOTICE data dep for //src/libraries/java/nv-boot-parent:NOTICE. The
Maven lock stays the single root //:maven_install.json (where the Java dep
versions live). This never ran before #311 enabled root tests.

Co-authored-by: Balaji Ganesan <bganesan@nvidia.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@balajinvda balajinvda changed the title test(ci): quarantine broken nv-boot notice_check_test to green main fix(nv-boot): make notice_check_test work in the monorepo layout Jul 21, 2026
@balajinvda
balajinvda force-pushed the fix/quarantine-notice-check branch from f4d7ea0 to 70cf875 Compare July 21, 2026 20:49
@balajinvda
balajinvda added this pull request to the merge queue Jul 21, 2026
Merged via the queue into main with commit d6c2206 Jul 21, 2026
13 checks passed
@balajinvda
balajinvda deleted the fix/quarantine-notice-check branch July 21, 2026 21:25
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.

nv-boot notice_check_test is broken in the monorepo layout (quarantined)

2 participants