Skip to content

Gate checkout-only tests with a root fixture - #7389

Open
diegoferigo-rai wants to merge 1 commit into
isaac-sim:release/3.0.0from
diegoferigo-rai:diegoferigo/skip-checkout-only-unit-tests
Open

Gate checkout-only tests with a root fixture#7389
diegoferigo-rai wants to merge 1 commit into
isaac-sim:release/3.0.0from
diegoferigo-rai:diegoferigo/skip-checkout-only-unit-tests

Conversation

@diegoferigo-rai

@diegoferigo-rai diegoferigo-rai commented Aug 27, 2026

Copy link
Copy Markdown

Description

A handful of unit tests inspect repository artifacts that only exist in a source checkout (apps, scripts, the root pyproject.toml, uv.lock, tools/wheel_builder, and the workflow files). Run from an installed package they fail because those paths are absent. This gates them behind a shared source_checkout_root session fixture that locates the checkout and skips the test when it is not present, and returns the root path so the tests stop recomputing it by hand.

The fixture lives in source/isaaclab/test/conftest.py. The affected tests now take source_checkout_root and drop their local _repo_root() helpers, so the gating is explicit at the test and the requirement is hard to forget: a checkout-only test needs the root anyway, so it asks for the fixture and gets the skip for free.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Release backport

  • Backport this pull request to the active release branch after it merges into develop

Checklist

  • I have read and understood the contribution guidelines
  • I have run the pre-commit checks with ./isaaclab.sh --format
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • I have added a changelog fragment under source/<pkg>/changelog.d/ for every touched package
  • I have added my name to the CONTRIBUTORS.md or my name already exists there

Several unit tests validate source-tree artifacts such as the root pyproject, uv.lock, tools, apps, and scripts. These tests are useful in the repository checkout, but they are not valid for installed-package test runs where those artifacts are intentionally absent.

Add a shared source checkout root fixture that skips when the checkout is unavailable and returns the root path when it is present. Use it in the affected tests so the same fixture provides both the gate and the path under test.
@github-actions github-actions Bot added documentation Improvements or additions to documentation asset New asset feature or request isaac-mimic Related to Isaac Mimic team infrastructure labels Aug 27, 2026
@diegoferigo-rai
diegoferigo-rai changed the base branch from develop to release/3.0.0 August 27, 2026 16:46
@diegoferigo-rai
diegoferigo-rai marked this pull request as ready for review August 27, 2026 16:51
@diegoferigo-rai
diegoferigo-rai requested a review from a team August 27, 2026 16:51
@greptile-apps

greptile-apps Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR introduces a shared session fixture that identifies a source checkout and skips checkout-only tests when repository artifacts are unavailable.

  • Replaces duplicated and fixed-depth repository-root discovery with source_checkout_root.
  • Gates tests that inspect root metadata, scripts, applications, workflows, and wheel-builder tools.
  • Patches the install module’s root for the Pink IK metadata test.
  • Adds a test-only changelog fragment.

Confidence Score: 5/5

The PR appears safe to merge, with no concrete blocking or independently actionable non-blocking defects identified.

The fixture consistently resolves repository artifacts in a normal source checkout and skips only when that checkout cannot be identified; the updated tests preserve their prior assertions and execution behavior.

Important Files Changed

Filename Overview
source/isaaclab/test/conftest.py Adds centralized source-checkout discovery and session-scoped skipping for tests requiring repository artifacts.
source/isaaclab/test/cli/test_teleop_entrypoints.py Uses the fixture root when validating teleoperation scripts and metadata while preserving production dispatch assertions.
source/isaaclab/test/cli/test_uv_run_pyproject.py Replaces repeated root discovery with fixture-injected paths for root metadata and workflow checks.
source/isaaclab/test/cli/test_wheel_builder_metadata.py Routes wheel-builder scripts and metadata inputs through the shared checkout root without changing subprocess semantics.
source/isaaclab/test/cli/test_install.py Patches the install module’s repository root so the checkout-only Pink IK stack test reads fixture-selected metadata.
source/isaaclab/test/app/test_experience_files.py Resolves application experience files through the shared checkout root.
source/isaaclab/test/test_scripts_warp_backward_ordering.py Gates static entry-script ordering checks through the shared checkout fixture.

Reviews (1): Last reviewed commit: "Gate checkout-only tests with a root fix..." | Re-trigger Greptile

@isaaclab-review-bot isaaclab-review-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Isaac Lab Review Bot

The PR centralizes source-checkout discovery in a session-scoped fixture and consistently migrates tests that inspect repository-only artifacts to skip when no checkout is available.

  • Design and architecture: Placing source_checkout_root in source/isaaclab/test/conftest.py makes it available to the affected app, CLI, and top-level tests while removing duplicated root-discovery logic. The three-marker check identifies the repository root rather than the package directory.
  • API: No production or public API changes are introduced. The modified helper signatures are private to test modules, their call sites are updated consistently, and the test-only .skip changelog fragment matches repository policy.
  • Implementation: The migrated paths for apps, scripts, root metadata, lock data, workflows, and wheel-builder tools are derived consistently from the fixture. The Pink IK test appropriately patches the production root constant while exercising production path resolution; the teleop tests separately validate dispatch and checkout artifact existence. A minor tradeoff is that checkout detection intentionally depends on isaaclab.sh, pyproject.toml, and source/ remaining stable repository markers.

No blocking issues. No inline issue met the actionable-evidence threshold; the assessment above records the review feedback.

Automated review; human maintainers own approval decisions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

asset New asset feature or request documentation Improvements or additions to documentation infrastructure isaac-mimic Related to Isaac Mimic team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant