feat(exit-codes): implement Robot Framework-style exit codes (#469) by oboehmer · Pull Request #553 · netascode/nac-test

oboehmer · 2026-02-19T16:25:15Z

Description

Implement Robot Framework-style exit codes for nac-test CLI to provide meaningful feedback for CI/CD pipelines. Exit codes now follow Robot Framework conventions: 0=pass, 1-249=failed test count, 250=250+ failures, 252=invalid Robot args/no tests, 253=interrupted, 255=internal error.
This also aligns to current nac-test 1.x behaviour

Closes

Fixes feat: nac-test return code should mimic previous behavior #469

Related Issue(s)

Type of Change

Bug fix (non-breaking change that fixes an issue)
New feature (non-breaking change that adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Refactoring / Technical debt (internal improvements with no user-facing changes)
Documentation update
Chore (build process, CI, tooling, dependencies)
Other (please describe):

Test Framework Affected

PyATS
Robot Framework
Both
N/A (not test-framework specific)

Network as Code (NaC) Architecture Affected

Platform Tested

macOS (version tested: darwin)
Linux (distro/version tested: )

Key Changes

Exit code constants: Add named constants (EXIT_ERROR, EXIT_FAILURE_CAP, EXIT_INTERRUPTED, EXIT_INVALID_ARGS, EXIT_INVALID_ROBOT_ARGS) replacing magic numbers
Single source of truth: Remove legacy errorhandler module; CombinedResults.exit_code is now the sole exit code authority
POSIX compliance: Split invalid args into EXIT_INVALID_ARGS=2 (nac-test CLI) and EXIT_INVALID_ROBOT_ARGS=252 (Robot Framework args)
Type-safe error handling: Add ErrorType enum (GENERIC, INVALID_ROBOT_ARGS, INTERRUPTED) replacing fragile string markers
Exit code priority: Implement correct priority ordering: 253 (interrupted) > 252 (invalid args) > 255 (generic error)
KeyboardInterrupt handling: Properly catch and translate to exit code 253
Centralized error reporting: All user-facing error/success output now in main.py with consistent styling (❌ errors to stderr, ⚠️ warnings, ✅ success). Orchestrators only log errors and store them in TestResults, following single-responsibility principle.
Documentation: Update README.md with exit code table

Testing Done

Unit tests added/updated
Integration tests performed
Manual testing performed:
- PyATS tests executed successfully
- Robot Framework tests executed successfully
- D2D/SSH tests executed successfully (if applicable)
- HTML reports generated correctly
All existing tests pass (pytest / pre-commit run -a)

Test Commands Used

uv run pytest tests/unit/cli/test_main_exit_codes.py tests/unit/core/test_types.py tests/unit/robot/test_orchestrator.py -n auto --dist loadscope -v
uv run pre-commit run --all-files

Checklist

Code follows project style guidelines (pre-commit run -a passes)
Self-review of code completed
Code is commented where necessary (especially complex logic)
Documentation updated (if applicable)
No new warnings introduced
Changes work on both macOS and Linux
CHANGELOG.md updated (if applicable)

Screenshots (if applicable)

N/A

Additional Notes

Exit Code Summary Table (from README):

Exit Code	Meaning	Description
0	Success	All tests passed, no errors
1-250	Test failures	Number of failed tests (capped at 250)
2	Invalid nac-test arguments	Invalid or conflicting nac-test CLI arguments
252	Invalid Robot Framework arguments or no tests found	Robot Framework invalid arguments or no tests executed
253	Execution interrupted	Test execution was interrupted (Ctrl+C, etc.)
255	Execution error	Framework crash or infrastructure error

Deviation from Robot Framework convention:

We use 2 for invalid nac-test CLI arguments instead of 252. This aligns with POSIX convention and is also necessary because Typer/Click hardcodes exit code 2 for CLI argument validation errors, and changing this behavior would require significant framework workarounds.
We do not use 251 (help/version display) as Typer handles these with exit code 0.

…ource of truth for exit codes ## Summary Removed the legacy errorhandler system that was interfering with proper exit code behavior. Exit codes are now handled exclusively through CombinedResults.exit_code, providing a single, predictable source of truth that follows Robot Framework conventions. ## Key Changes **Errorhandler Removal:** - Eliminated errorhandler dependency that was causing logger.error() calls to override intended exit codes - Restored proper logger.error() usage throughout the codebase (no more workarounds with logger.warning()) - Developers can now use logger.error() naturally without unintended side effects on exit codes **Single Source of Truth:** - CombinedResults.exit_code is now the only exit code calculation method - Removed unused TestResults.exit_code method that was never called in production - Simplified architecture: exit codes are purely a CLI aggregation concern, not individual framework concern ## Benefits - **Predictable Behavior**: Exit codes come from one clear path (CombinedResults → CLI) - **No Hidden Side Effects**: Logging levels don't accidentally change exit codes - **Cleaner Architecture**: No duplicate exit code logic across TestResults and CombinedResults - **Developer Friendly**: logger.error() works as expected without exit code interference ## Tests - **New CLI exit code tests**: Comprehensive test suite for main.py exit code behavior (9 tests) - **Removed**: Unused TestResults.exit_code tests that tested non-production code paths - All existing integration and E2E tests continue to pass The errorhandler was a legacy mechanism from before CombinedResults existed. With proper structured error handling now in place, the errorhandler created more problems than it solved by interfering with the intended graduated exit code system.

Introduce symbolic exit code constants in nac_test/core/constants.py following Robot Framework conventions: EXIT_FAILURE_CAP = 250 # max test failure count reported EXIT_INVALID_ARGS = 252 # invalid RF args or no tests found EXIT_INTERRUPTED = 253 # Ctrl+C / interrupted execution EXIT_ERROR = 255 # infrastructure/execution errors EXIT_SUCCESS (0) is deliberately not defined — it is a universal POSIX convention that never changes, so a named constant adds no clarity. Removed the spuriously added __all__ from constants.py. Also fix template rendering error handling: instead of propagating exceptions in render-only mode (which leaked as exit code 1, violating the "1-250 = test failure count" convention), rendering failures are now converted to TestResults.from_error() objects with descriptive messages. main.py inspects the resulting CombinedResults.has_errors to exit with EXIT_ERROR (255) and display an actionable "❌ Template rendering failed: <reason>" message, consistent with all other infrastructure failures. Updated all tests to use the new constants instead of magic numbers, except where literals are derived directly from test input (e.g. `failed=3` → `assert exit_code == 3`) and therefore more readable as literals.

…t (252) EXIT_INVALID_ARGS was previously 252 (Robot Framework convention). This was confusing because nac-test's own CLI argument errors should follow POSIX/Typer conventions (exit code 2) rather than RF conventions. Split into two distinct constants: - EXIT_INVALID_ARGS = 2 (nac-test CLI arg errors, aligns with POSIX/Typer) - EXIT_INVALID_ROBOT_ARGS = 252 (Robot Framework invalid args / no tests found) This allows CI/CD to unambiguously distinguish exit code 2 from a bad CLI invocation vs. 2 test failures (which would produce artifacts). Updated all production code, tests, and README accordingly.

…type-safe exit code determination Replace fragile string-based error detection (ERROR_MARKER_*) with a typed ErrorType enum approach. This enables compile-time checking and explicit exit code priority handling. Changes: - Add ErrorType enum (GENERIC, INVALID_ROBOT_ARGS, INTERRUPTED) - Add error_type field to TestResults dataclass - Update from_error() to accept optional error_type parameter - Fix exit code priority: 253 (interrupted) > 252 (invalid args) > 255 (generic) - Remove ERROR_MARKER_* constants from constants.py - Add unit tests for ErrorType and exit code priority

Move all user-facing error/success output to main.py: - Display ❌ for errors, ⚠️ for warnings, ✅ for success - Route all error messages to stderr consistently Remove redundant error output from orchestrators: - robot/orchestrator.py: remove typer.echo calls (-15 lines) - combined_orchestrator.py: simplify exception handler (-17 lines) Orchestrators now only log errors and store them in TestResults, following single-responsibility principle.

Tests previously expected exit code 1 for controller detection failures. With the new exit code scheme, infrastructure errors return EXIT_ERROR (255).

aitestino

Hey Oliver, @oboehmer - good work with this. Thank you.

A couple of things at quick glance that stood out to me:

1. Unused `ERROR_TYPE_EXIT_CODES` Dict

Location: nac_test/core/types.py:44-50

The ERROR_TYPE_EXIT_CODES mapping is defined but never used. The CombinedResults.exit_code property uses inline if/elif checks instead of looking up from this dict:

# Dict is defined but unused:
ERROR_TYPE_EXIT_CODES: dict[ErrorType, int] = {
    ErrorType.INVALID_ROBOT_ARGS: EXIT_INVALID_ROBOT_ARGS,
    ErrorType.INTERRUPTED: EXIT_INTERRUPTED,
    ErrorType.GENERIC: EXIT_ERROR,
}

# exit_code property doesn't use it:
if ErrorType.INTERRUPTED in error_types:
    return EXIT_INTERRUPTED
if ErrorType.INVALID_ROBOT_ARGS in error_types:
    return EXIT_INVALID_ROBOT_ARGS
return EXIT_ERROR

This creates two sources of truth that could drift. Two options:

Remove the dict (my preference) - With only 3 error types, the explicit if chain is more readable and the priority ordering is immediately visible. The dict would only add value if we had many error types; with three, it's unnecessary indirection that adds a maintenance burden without improving clarity.
Use the dict - Replace the if/elif chain with a priority-ordered lookup

What's your thinking on keeping vs. removing this mapping?

2. `errorhandler` Dependency in pyproject.toml

Location: pyproject.toml:52

All imports and usages of errorhandler have been removed from the code, but errorhandler>=2.0 is still in pyproject.toml. This should be removed to avoid shipping an unused dependency.

Minor Observations (Non-Blocking)

Naming: `EXIT_INVALID_ROBOT_ARGS` Dual Purpose

The constant is used for both "invalid Robot arguments" AND "no tests found" (via is_empty check in CombinedResults.exit_code). The name suggests it's exclusively about invalid arguments. Consider renaming to EXIT_DATA_ERROR (matching Robot Framework's naming) or adding a clarifying comment. Not critical since the docstring explains the dual meaning.

Priority Ordering Documentation

The docstring says "Priority: 253 > 252 > 255" which looks counterintuitive numerically. A brief note about why this ordering (interrupted = most actionable signal for CI/CD) would help future maintainers who might assume lower numbers = higher priority.

P.S. — This comment was drafted using voice-to-text via Claude Code. If the tone comes across as overly direct or terse, please know that's just how it tends to phrase things. No offense or criticism is intended — this is purely an objective technical review of the PR. Thanks for understanding! 🙂

With only 3 error types, the explicit if chain in CombinedResults.exit_code is more readable and makes priority ordering immediately visible. The dict was defined but never used, creating two sources of truth that could drift.

The errorhandler package is no longer used in the codebase after centralizing exit code handling in CombinedResults.exit_code.

Rename to match Robot Framework's naming convention for exit code 252. This constant serves dual purpose: invalid Robot arguments AND no tests found (via is_empty check), so the more generic name is more accurate.

…cstring Add explanation for why priority is 253 > 252 > 255 (counterintuitive numerically): interrupted is most actionable for CI/CD, data errors indicate config issues, generic errors may be transient.

…69-exit-code Resolve merge conflict in combined_orchestrator.py: - Keep render_only check from base branch (skip controller detection in render-only mode) - Use EXIT_ERROR constant instead of hardcoded 1 for consistent exit codes

… check happy

oboehmer · 2026-02-23T07:54:12Z

thanks for the catches in your review, I addressed all comments, please re-review, @aitestino

…eta' into feat/469-exit-code

aitestino

Thanks! Great job. I upon a 2nd glance spotted these but they are truly nitpicks, opened the issues as to not look the other way...but this is good to merge imo:

P.S. — This comment was drafted using voice-to-text. If the tone comes across as overly direct or terse, please know that's just how it tends to phrase things. No offense or criticism is intended — this is purely an objective technical review of the PR. Thanks for understanding! 🙂

…553) * refactor: remove legacy errorhandler, use CombinedResults as single source of truth for exit codes ## Summary Removed the legacy errorhandler system that was interfering with proper exit code behavior. Exit codes are now handled exclusively through CombinedResults.exit_code, providing a single, predictable source of truth that follows Robot Framework conventions. ## Key Changes **Errorhandler Removal:** - Eliminated errorhandler dependency that was causing logger.error() calls to override intended exit codes - Restored proper logger.error() usage throughout the codebase (no more workarounds with logger.warning()) - Developers can now use logger.error() naturally without unintended side effects on exit codes **Single Source of Truth:** - CombinedResults.exit_code is now the only exit code calculation method - Removed unused TestResults.exit_code method that was never called in production - Simplified architecture: exit codes are purely a CLI aggregation concern, not individual framework concern ## Benefits - **Predictable Behavior**: Exit codes come from one clear path (CombinedResults → CLI) - **No Hidden Side Effects**: Logging levels don't accidentally change exit codes - **Cleaner Architecture**: No duplicate exit code logic across TestResults and CombinedResults - **Developer Friendly**: logger.error() works as expected without exit code interference ## Tests - **New CLI exit code tests**: Comprehensive test suite for main.py exit code behavior (9 tests) - **Removed**: Unused TestResults.exit_code tests that tested non-production code paths - All existing integration and E2E tests continue to pass The errorhandler was a legacy mechanism from before CombinedResults existed. With proper structured error handling now in place, the errorhandler created more problems than it solved by interfering with the intended graduated exit code system. * refactor(exit-codes): replace hardcoded exit codes with named constants Introduce symbolic exit code constants in nac_test/core/constants.py following Robot Framework conventions: EXIT_FAILURE_CAP = 250 # max test failure count reported EXIT_INVALID_ARGS = 252 # invalid RF args or no tests found EXIT_INTERRUPTED = 253 # Ctrl+C / interrupted execution EXIT_ERROR = 255 # infrastructure/execution errors EXIT_SUCCESS (0) is deliberately not defined — it is a universal POSIX convention that never changes, so a named constant adds no clarity. Removed the spuriously added __all__ from constants.py. Also fix template rendering error handling: instead of propagating exceptions in render-only mode (which leaked as exit code 1, violating the "1-250 = test failure count" convention), rendering failures are now converted to TestResults.from_error() objects with descriptive messages. main.py inspects the resulting CombinedResults.has_errors to exit with EXIT_ERROR (255) and display an actionable "❌ Template rendering failed: <reason>" message, consistent with all other infrastructure failures. Updated all tests to use the new constants instead of magic numbers, except where literals are derived directly from test input (e.g. `failed=3` → `assert exit_code == 3`) and therefore more readable as literals. * refactor(exit-codes): split EXIT_INVALID_ARGS into POSIX (2) and Robot (252) EXIT_INVALID_ARGS was previously 252 (Robot Framework convention). This was confusing because nac-test's own CLI argument errors should follow POSIX/Typer conventions (exit code 2) rather than RF conventions. Split into two distinct constants: - EXIT_INVALID_ARGS = 2 (nac-test CLI arg errors, aligns with POSIX/Typer) - EXIT_INVALID_ROBOT_ARGS = 252 (Robot Framework invalid args / no tests found) This allows CI/CD to unambiguously distinguish exit code 2 from a bad CLI invocation vs. 2 test failures (which would produce artifacts). Updated all production code, tests, and README accordingly. * refactor(exit-codes): replace string markers with ErrorType enum for type-safe exit code determination Replace fragile string-based error detection (ERROR_MARKER_*) with a typed ErrorType enum approach. This enables compile-time checking and explicit exit code priority handling. Changes: - Add ErrorType enum (GENERIC, INVALID_ROBOT_ARGS, INTERRUPTED) - Add error_type field to TestResults dataclass - Update from_error() to accept optional error_type parameter - Fix exit code priority: 253 (interrupted) > 252 (invalid args) > 255 (generic) - Remove ERROR_MARKER_* constants from constants.py - Add unit tests for ErrorType and exit code priority * refactor(exit-codes): centralize error reporting in main.py Move all user-facing error/success output to main.py: - Display ❌ for errors, ⚠️ for warnings, ✅ for success - Route all error messages to stderr consistently Remove redundant error output from orchestrators: - robot/orchestrator.py: remove typer.echo calls (-15 lines) - combined_orchestrator.py: simplify exception handler (-17 lines) Orchestrators now only log errors and store them in TestResults, following single-responsibility principle. * test: update controller detection tests to expect EXIT_ERROR (255) Tests previously expected exit code 1 for controller detection failures. With the new exit code scheme, infrastructure errors return EXIT_ERROR (255). * Fix test comment with correct expected exit code * refactor(exit-codes): remove unused ERROR_TYPE_EXIT_CODES dict With only 3 error types, the explicit if chain in CombinedResults.exit_code is more readable and makes priority ordering immediately visible. The dict was defined but never used, creating two sources of truth that could drift. * chore(deps): remove unused errorhandler dependency The errorhandler package is no longer used in the codebase after centralizing exit code handling in CombinedResults.exit_code. * refactor(exit-codes): rename EXIT_INVALID_ROBOT_ARGS to EXIT_DATA_ERROR Rename to match Robot Framework's naming convention for exit code 252. This constant serves dual purpose: invalid Robot arguments AND no tests found (via is_empty check), so the more generic name is more accurate. * docs(exit-codes): explain priority ordering rationale in exit_code docstring Add explanation for why priority is 253 > 252 > 255 (counterintuitive numerically): interrupted is most actionable for CI/CD, data errors indicate config issues, generic errors may be transient. * tests: add defaults.apic.version to e2e fixtures to make ACI defaults check happy

oboehmer added 4 commits February 19, 2026 09:52

oboehmer requested review from aitestino and danischm February 19, 2026 16:25

oboehmer self-assigned this Feb 19, 2026

oboehmer added 2 commits February 19, 2026 18:04

test: update controller detection tests to expect EXIT_ERROR (255)

813717a

Tests previously expected exit code 1 for controller detection failures. With the new exit code scheme, infrastructure errors return EXIT_ERROR (255).

oboehmer added enhancement New feature or request new-infra Issues related to the new pyats/robot infra currently under development labels Feb 20, 2026

Fix test comment with correct expected exit code

f3a86b2

aitestino requested changes Feb 22, 2026

View reviewed changes

oboehmer added 6 commits February 23, 2026 08:14

chore(deps): remove unused errorhandler dependency

93da9a0

The errorhandler package is no longer used in the codebase after centralizing exit code handling in CombinedResults.exit_code.

refactor(exit-codes): rename EXIT_INVALID_ROBOT_ARGS to EXIT_DATA_ERROR

a88f1de

Rename to match Robot Framework's naming convention for exit code 252. This constant serves dual purpose: invalid Robot arguments AND no tests found (via is_empty check), so the more generic name is more accurate.

docs(exit-codes): explain priority ordering rationale in exit_code do…

29f9517

…cstring Add explanation for why priority is 253 > 252 > 255 (counterintuitive numerically): interrupted is most actionable for CI/CD, data errors indicate config issues, generic errors may be transient.

tests: add defaults.apic.version to e2e fixtures to make ACI defaults…

d67b633

… check happy

oboehmer mentioned this pull request Feb 23, 2026

ci: test PRs against merge commit to catch integration issues #567

Closed

oboehmer requested a review from aitestino February 23, 2026 07:54

Merge remote-tracking branch 'origin/release/pyats-integration-v1.1-b…

59d3f9e

…eta' into feat/469-exit-code

aitestino approved these changes Feb 26, 2026

View reviewed changes

oboehmer merged commit 53b1447 into release/pyats-integration-v1.1-beta Feb 26, 2026
7 checks passed

oboehmer deleted the feat/469-exit-code branch February 26, 2026 05:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(exit-codes): implement Robot Framework-style exit codes (#469)#553

feat(exit-codes): implement Robot Framework-style exit codes (#469)#553
oboehmer merged 14 commits intorelease/pyats-integration-v1.1-betafrom
feat/469-exit-code

oboehmer commented Feb 19, 2026 •

edited

Loading

Uh oh!

aitestino left a comment

Uh oh!

oboehmer commented Feb 23, 2026

Uh oh!

aitestino left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

oboehmer commented Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Closes

Related Issue(s)

Type of Change

Test Framework Affected

Network as Code (NaC) Architecture Affected

Platform Tested

Key Changes

Testing Done

Test Commands Used

Checklist

Screenshots (if applicable)

Additional Notes

Uh oh!

aitestino left a comment

Choose a reason for hiding this comment

1. Unused ERROR_TYPE_EXIT_CODES Dict

2. errorhandler Dependency in pyproject.toml

Minor Observations (Non-Blocking)

Naming: EXIT_INVALID_ROBOT_ARGS Dual Purpose

Priority Ordering Documentation

Uh oh!

oboehmer commented Feb 23, 2026

Uh oh!

aitestino left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

oboehmer commented Feb 19, 2026 •

edited

Loading

1. Unused `ERROR_TYPE_EXIT_CODES` Dict

2. `errorhandler` Dependency in pyproject.toml

Naming: `EXIT_INVALID_ROBOT_ARGS` Dual Purpose