fix(cli): validate required args before running --diagnostic#741
Open
fix(cli): validate required args before running --diagnostic#741
Conversation
- Add early `zip` preflight in diagnostic collection script (with install hints) - Hard-error on Windows when `--diagnostic` is used (exit code 2) - Clarify `--diagnostic` help text as Linux/macOS-only - Add unit test to ensure Windows gating avoids running subprocess
Handle --diagnostic inside main() after Typer/Click parsing so missing required options (-d/-t) fail normally. Run the diagnostic script via run_diagnostic() and exit with its return code; hard-error on Windows.
Add targeted `type: ignore[unreachable]` on assertions after calling run_diagnostic (annotated NoReturn) inside pytest.raises blocks, so the pre-commit mypy hook passes.
Introduce base_argv/base_argv_with_spaces fixtures and tighten reconstruct tests to assert shlex round-trips exactly. Reuse base_argv in TestRunDiagnostic to avoid ad-hoc argv construction.
Create the diagnostic zip from the parent directory so the archive contains only the diagnostic folder (no full /var/... path prefixes).
Require the nac-test command to be passed as a single quoted argument ($1) and fail fast otherwise. Also clarify the log message that nac-test output is not shown on screen and is written to 100_nac_test_execution.txt.
Compute Robot Framework output file list once from $OUTPUT_DIR/robot_results and reuse it for both logging and copying, avoiding duplicate filesystem scans.
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.
Description
This PR improves
--diagnosticbehavior and the bundled diagnostic collection script to make diagnostics safer, more accurate, and easier to consume.CLI
--diagnosticbehavior (Fixes #740)--diagnostichandling out of the Typer callback and intonac_test/cli/main.pyso required args are validated by Typer/Click before diagnostics run.run_diagnostic(...)(annotatedNoReturn) that runs the diagnostic script and exits with the script’s return code.--diagnostic.Diagnostic script improvements (Fixes #664 + follow-ups)
zipcommand is missing (observed on WSL2 environment)PIPESTATUSusage)./var/...prefixes in the archive contents).pyats version checkinstead ofpyats.__version__for reliable version capture.$1) and enforce the contract.$OUTPUT_DIR/robot_results, and collect robot outputs via a singlefindpass.Tests
tests/unit/cli/test_diagnostic.pyto match the newrun_diagnosticentrypoint._reconstruct_commandcoverage with strictshlex.join/shlex.splitround-trip assertions.NoReturninpytest.raises.Closes
Type of Change
Test Framework Affected
NaC Architecture Affected
Platform Tested
Key Changes
--diagnosticno longer bypasses missing options).Testing Done
pytest/pre-commit run -a)Test Commands Used
Additional Notes
shlex.join), and the script enforces that it receives exactly one command argument.$OUTPUT_DIR/robot_results.