Skip to content

cli: Fail YAML validation for missing input - #796

Merged
ximion merged 2 commits into
ximion:mainfrom
eugeniodepalo:fix/yaml-missing-input-status
Oct 15, 2026
Merged

ximion merged 2 commits into
ximion:mainfrom
eugeniodepalo:fix/yaml-missing-input-status

Conversation

@eugeniodepalo

Copy link
Copy Markdown
Contributor

YAML validation returned success for missing files because its integer status
function returned FALSE.

Return ASCLI_EXIT_CODE_BAD_INPUT, matching text validation. Maintainer-mode build and tests pass.

ascli_validate_files_format() returns a process status, but its YAML
missing-file path returned FALSE and therefore reported success.

Return ASCLI_EXIT_CODE_BAD_INPUT, matching text validation.
Copilot AI balanced review requested due to automatic review settings October 15, 2026 10:13
@github-actions github-actions Bot added tools AppStream's CLI tools tests labels Oct 15, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The focused fix aligns validation behavior and includes appropriate regression coverage, with no unresolved findings.

0 open findings

What changed in this PR

Corrects YAML validation’s exit status for missing input files, matching text validation behavior.

Changes:

  • Returns ASCLI_EXIT_CODE_BAD_INPUT instead of success.
  • Adds regression coverage for missing files in text and YAML modes.
  • Registers the CLI test with Meson.
File Description
tools/​ascli-actions-validate.c Corrects the missing-file exit code.
tests/​test-cli.c Tests missing-file exit status in both formats.
tests/​meson.build Builds and registers the CLI test.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@ximion

ximion commented Oct 15, 2026

Copy link
Copy Markdown
Owner

Why the heck is Copilot here? I didn't enable that...

Please drop the test, it's very verbose and doesn't actually give us much. Do keep the bugfix though, that one is valid!

@eugeniodepalo
eugeniodepalo force-pushed the fix/yaml-missing-input-status branch from 2779ace to a598ef1 Compare October 15, 2026 17:55
@github-actions github-actions Bot removed the tests label Oct 15, 2026
@eugeniodepalo

Copy link
Copy Markdown
Contributor Author

Thanks for the feedback @ximion - I've deleted the test.

@ximion
ximion merged commit 7732512 into ximion:main Oct 15, 2026
10 checks passed
@ximion

ximion commented Oct 15, 2026

Copy link
Copy Markdown
Owner

Thank you for the patch! :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tools AppStream's CLI tools

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants