Skip to content

Return non-zero exit status when validation fails - #25

Merged
olivhoenen merged 2 commits into
iterorganization:developfrom
chupakobra6:fix/cli-validation-exit-status
Oct 1, 2026
Merged

olivhoenen merged 2 commits into
iterorganization:developfrom
chupakobra6:fix/cli-validation-exit-status

Conversation

@chupakobra6

Copy link
Copy Markdown
Contributor

Closes #22.

The CLI currently reports failed validation while returning exit status 0, so CI systems treat rejected datasets as successful checks.

Return an explicit status from main() and propagate it through sys.exit() at both CLI entry paths:

  • 0 when every validated URI passes;
  • 1 when any validated URI fails;
  • 2 for argument and unrecognized-command errors.

The status is computed across every executed ValidateCommand. Per-URI and summary reports are still generated before returning, and the existing final stdout line listing failed URIs is unchanged. Document the exit-status contract in the README.

Add regression coverage for successful validation, mixed multi-URI validation, report generation on failure, failed-URI output, argparse errors, and the real python -m imas_validator entry point with an invalid netCDF dataset.

Validation on the rebased current head:

  • Full Python 3.13 suite: 189 passed, 2 existing skips.
  • Focused CLI scenarios: 5 passed.
  • Black, Flake8, MyPy, and isort pass.

Comment thread tests/test_cli.py Outdated

assert completed.returncode == 1
assert completed.stdout.splitlines()[-1] == str(uri)
assert "FAILED validation." in completed.stdout

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Finding from Claude:

The check assert "FAILED validation." in completed.stdout depends on how long the temp path is. The "URI has FAILED validation." line is printed by rich's RichHandler. When stdout isn't a terminal, rich wraps lines at 80 columns by default, and it can break between "FAILED" and "validation.". That happens or not depending on how long the pytest temp path is.

It can therefore fail on CI, under -n=auto (whose worker subfolders make the path longer), or for users with longer usernames. Possible fixes:

  • drop this check;
  • or match on a normalised string, for example " ".join(stdout.split()).

@chupakobra6
chupakobra6 requested a review from olivhoenen October 1, 2026 12:20

@olivhoenen olivhoenen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks!

@olivhoenen
olivhoenen merged commit dc0fbdb into iterorganization:develop Oct 1, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CLI exits 0 even when validation FAILED, so it cannot be used as a CI gate

2 participants