Skip to content

Fix AppLauncher tests: update newton alias and remove legacy headless CLI test - #7533

Merged
kellyguo11 merged 2 commits into
isaac-sim:developfrom
ChaitanyaParate:chaitanya/fix-app-launcher-tests
Sep 4, 2026
Merged

Fix AppLauncher tests: update newton alias and remove legacy headless CLI test#7533
kellyguo11 merged 2 commits into
isaac-sim:developfrom
ChaitanyaParate:chaitanya/fix-app-launcher-tests

Conversation

@ChaitanyaParate

@ChaitanyaParate ChaitanyaParate commented Sep 3, 2026

Copy link
Copy Markdown

This PR isolates and fixes the failing tests in test_kwarg_launch.py that surfaced during the investigation of #7403 (and the closed PR #7444).

When run on a clean upstream/develop, test_kwarg_launch.py fails on exactly four tests. This PR addresses them:

  1. Newton Alias Fallout: Fixed three tests (test_parse_visualizer_csv_accepts_comma_delimited_values, test_visualizer_csv_does_not_swallow_hydra_overrides, and test_matrix_cli_kit_newton_with_custom_kit_cfg_intent_non_headless) which were failing because they asserted against the deprecated newton alias instead of newton_gl. The third test was explicitly renamed to test_matrix_cli_kit_newton_gl_with_custom_kit_cfg_intent_non_headless to reflect the updated assertion.
  2. Removed Dead CLI Path: Deleted test_matrix_headless_with_viz_names_takes_precedence. This test was failing because it passed headless=True directly through _resolve_visualizer_settings, simulating the --headless CLI flag which was removed in 3.0 and is no longer supported upstream.
  3. Hygiene Updates (Not Failing):
    • Replaced "kit, newton" with "kit, newton_gl" in test_parse_visualizer_csv_rejects_spaces_between_entries. This test successfully raises an ArgumentTypeError before checking aliases so it wasn't failing, but it was updated to prevent stale strings from persisting.
    • Renamed test_matrix_no_cli_with_cfg_kit_newton_non_headless to test_matrix_no_cli_with_cfg_kit_newton_gl_non_headless for naming consistency. This test also passed previously but carried the stale alias in its name.

See the attached pytest_before_fix.txt (showing the 4 failures on develop) and pytest_after_fix.txt (showing 47 clean passes) for the exact run logs.

pytest_after_fix.txt
pytest_before_fix.txt

Release backport

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

@ChaitanyaParate
ChaitanyaParate requested a review from a team September 3, 2026 10:12
@github-actions github-actions Bot added bug Something isn't working isaac-lab Related to Isaac Lab team labels Sep 3, 2026

@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

This test-only change updates visualizer cases from the deprecated newton alias to newton_gl and removes one legacy combined headless/visualizer test. No production behavior is modified.

  • Design and architecture: The parsing, Hydra passthrough, and headless-resolution test paths remain aligned with the current visualizer naming. The change does not alter AppLauncher architecture or responsibility boundaries.
  • API: No public or extension-facing API is changed. Existing AppLauncher parsing and resolution methods are exercised with updated test inputs but are not redefined.
  • Implementation: The changed assertions consistently match the updated newton_gl inputs. Removing the combined deprecated-headless/visualizer case reduces coverage of that specific interaction, but the retained deprecated-headless precedence test continues to cover the legacy argument path, and the patch supplies no evidence that the removed combination remains supported.

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.

@greptile-apps

greptile-apps Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Updates AppLauncher tests to use the canonical newton_gl visualizer name and removes an obsolete test for the retired headless CLI path.

  • Replaces stale newton test inputs and expectations with newton_gl.
  • Renames matrix tests for consistency with the canonical alias.
  • Deletes the legacy headless-plus-visualizer precedence test.

Confidence Score: 5/5

The PR appears safe to merge because the test updates match the current AppLauncher visualizer contract and introduce no production behavior changes.

The canonical newton_gl value is supported by the parser and resolver, deprecated newton compatibility remains covered elsewhere, and the removed test asserted obsolete behavior.

Important Files Changed

Filename Overview
source/isaaclab/test/app/test_kwarg_launch.py Test-only updates align visualizer assertions with the canonical alias and remove a stale test for unsupported legacy CLI behavior.

Reviews (1): Last reviewed commit: "Fix AppLauncher tests: update newton ali..." | Re-trigger Greptile

@ChaitanyaParate
ChaitanyaParate force-pushed the chaitanya/fix-app-launcher-tests branch from bf16c94 to 799d709 Compare September 3, 2026 11:05
@kellyguo11

Copy link
Copy Markdown
Contributor

run-ci

@isaaclab-bot isaaclab-bot Bot added ci:run-docker Trigger the on-demand Docker and GPU CI workflow and removed ci:run-docker Trigger the on-demand Docker and GPU CI workflow labels Sep 3, 2026
Comment thread source/isaaclab/test/app/test_kwarg_launch.py Outdated
Signed-off-by: Kelly Guo <kellyg@nvidia.com>
@kellyguo11

Copy link
Copy Markdown
Contributor

run-ci

@isaaclab-bot isaaclab-bot Bot added ci:run-docker Trigger the on-demand Docker and GPU CI workflow and removed ci:run-docker Trigger the on-demand Docker and GPU CI workflow labels Sep 4, 2026
@kellyguo11
kellyguo11 merged commit cb43dd0 into isaac-sim:develop Sep 4, 2026
51 of 52 checks passed
@isaaclab-bot

isaaclab-bot Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Backported to release/3.0.0 as 1d072ba.

isaaclab-bot Bot pushed a commit that referenced this pull request Sep 4, 2026
… CLI test (#7533)

This PR isolates and fixes the failing tests in `test_kwarg_launch.py`
that surfaced during the investigation of #7403 (and the closed PR
#7444).

When run on a clean `upstream/develop`, `test_kwarg_launch.py` fails on
exactly four tests. This PR addresses them:
1. **Newton Alias Fallout:** Fixed three tests
(`test_parse_visualizer_csv_accepts_comma_delimited_values`,
`test_visualizer_csv_does_not_swallow_hydra_overrides`, and
`test_matrix_cli_kit_newton_with_custom_kit_cfg_intent_non_headless`)
which were failing because they asserted against the deprecated `newton`
alias instead of `newton_gl`. The third test was explicitly renamed to
`test_matrix_cli_kit_newton_gl_with_custom_kit_cfg_intent_non_headless`
to reflect the updated assertion.
2. **Removed Dead CLI Path:** Deleted
`test_matrix_headless_with_viz_names_takes_precedence`. This test was
failing because it passed `headless=True` directly through
`_resolve_visualizer_settings`, simulating the `--headless` CLI flag
which was removed in 3.0 and is no longer supported upstream.
3. **Hygiene Updates (Not Failing):** 
- Replaced `"kit, newton"` with `"kit, newton_gl"` in
`test_parse_visualizer_csv_rejects_spaces_between_entries`. This test
successfully raises an `ArgumentTypeError` before checking aliases so it
wasn't failing, but it was updated to prevent stale strings from
persisting.
- Renamed `test_matrix_no_cli_with_cfg_kit_newton_non_headless` to
`test_matrix_no_cli_with_cfg_kit_newton_gl_non_headless` for naming
consistency. This test also passed previously but carried the stale
alias in its name.

_See the attached `pytest_before_fix.txt` (showing the 4 failures on
develop) and `pytest_after_fix.txt` (showing 47 clean passes) for the
exact run logs._



[pytest_after_fix.txt](https://github.com/user-attachments/files/31785416/pytest_after_fix.txt)

[pytest_before_fix.txt](https://github.com/user-attachments/files/31785426/pytest_before_fix.txt)

## Release backport

- [x] <!-- backport-active-release --> Backport this pull request to the
active release branch after it merges into `develop`

---------

Signed-off-by: Kelly Guo <kellyg@nvidia.com>
Co-authored-by: Kelly Guo <kellyg@nvidia.com>

(cherry picked from commit cb43dd0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working isaac-lab Related to Isaac Lab team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants