Add test coverage for the nflow CLI - #12
Open
Anilreddy2309 wants to merge 1 commit into
Open
Conversation
…ate, stage-info) nvflow/cli/main.py had zero dedicated test coverage -- tests/test_cli_cmd.py tests a different module (nvflow/lib/cli_cmd.py, a shell-command builder) with a similarly-named file, which is easy to mistake for CLI coverage. Add tests/test_cli_main.py using typer.testing.CliRunner, exercised against the real auto-discovered stage registry via the `example` recipe (which needs none of the heavy nemo-skills/torch stack, so it registers even in the CI-lightweight test environment). Covers the happy path and the main error paths for each command: unknown recipe/workflow/stage, invalid stage-info path formats, and missing config files. Add `typer` to tests/requirements-ci.txt so this file is actually collected and run in CI rather than silently needing typer at collection time with nothing to provide it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Anil Balireddy <anilbalireddi@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
nvflow/cli/main.py(thelist-stages,run,run-all,validate,stage-infocommands) had zero dedicated test coverage.tests/test_cli_cmd.pytests a different module (nvflow/lib/cli_cmd.py, a shell-command builder) with a similarly-named file — easy to mistake for CLI coverage, but it isn't.This PR adds
tests/test_cli_main.py, usingtyper.testing.CliRunneragainst the real Typer app and the real auto-discovered stage registry (via theexamplerecipe, which needs none of the heavynemo-skills/torchstack, so it registers correctly even in the CI-lightweight test environment described intests/requirements-ci.txt).Coverage:
versionlist-stages: no args,--recipe, unknown recipe, unknown workflow,--config, missing config filestage-info: full path, short name +--recipe/--workflow, short name missing those options, invalid path format, unknown stagevalidate: valid config, missing config file, missing--configoptionrun/run-all: missing config file, missing--configoptionAlso adds
typertotests/requirements-ci.txt— without it, this test file can't even be collected in the lightweight CI environment (nvflow.cli.mainimportstyperdirectly), so this was necessary for the new tests to actually run in CI rather than error at collection.Test plan
ruff check/ruff format --checkpassmypypasses on the new filepip install -e . --no-deps+tests/requirements-ci.txt, nonemo-skills/torch)pytest tests/suite still passes (371 passed, 3 pre-existing skips, unrelated to this change) in the same environment -- confirms no regression🤖 Generated with Claude Code