Skip to content

Fix #507 --framework option for darnit run - #517

Closed
Natnaeltewodros wants to merge 2 commits into
darnitdevorg:mainfrom
Natnaeltewodros:fix-run-framework-option
Closed

Natnaeltewodros wants to merge 2 commits into
darnitdevorg:mainfrom
Natnaeltewodros:fix-run-framework-option

Conversation

@Natnaeltewodros

@Natnaeltewodros Natnaeltewodros commented Sep 28, 2026 •

Copy link
Copy Markdown

Summary

Fixes the darnit run command so that the --framework option is recognized and accepted. This allows users to explicitly select a framework, such as testchecks, when running the command.

Added a regression test to verify that darnit run --framework testchecks --feedback noninteractive is accepted successfully.

Type of Change

  • [ x] Bug fix (non-breaking change fixing an issue)
  • New feature (non-breaking change adding functionality)
  • Breaking change (fix or feature causing existing functionality to change)
  • Documentation update
  • Refactoring (no functional changes)

Framework Changes Checklist

If this PR modifies the darnit framework (packages/darnit/):

  • Updated framework spec (docs/architecture/framework-design.md) if behavior changed
  • Ran uv run python scripts/validate_sync.py --verbose and it passes

Control/TOML Changes Checklist

If this PR modifies controls or TOML configuration:

  • Control metadata defined in TOML (not Python code)
  • SARIF fields (description, severity, help_url) included where appropriate
  • Ran validation to confirm TOML schema compliance

Testing

  • [ x] Tests pass locally (uv run pytest tests/ -v)
  • [ x] Added tests for new functionality (if applicable)
  • [x ] Linting passes (uv run ruff check .)

AI assistance

  • No AI assistance was used
  • AI assistance was used

Additional Notes

@mlieberman85 mlieberman85 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for picking this up! The cli.py change is right: cmd_run already reads args.framework, it just was never registered on the run subparser. A few things before we can merge:

  1. Please link the issue: add Fixes #507 to the description.
  2. AI disclosure: the "AI assistance" section of the PR template is missing. Please fill it in. If AI was used, say which tool and which parts, and add an Assisted-by: trailer to the commit.
  3. Unrelated change: the PR re-indents the body of test_golden_prints_header. Please revert that so the diff only contains the new option and its test.
  4. Test placement: the new test is appended directly after the raise NotImplementedError of the previous test, with no blank lines before it and two stray blank lines inside it. Please separate it with two blank lines and run uv run ruff check . and uv run ruff format on the file.
  5. Test strength: the test only checks the exit code and empty stderr, which would also pass if --framework were accepted but ignored. Please also assert that the selected framework was used, for example that output includes a control ID that only the testchecks framework defines.

Once those are in, I'll approve the CI run.

@mlieberman85

Copy link
Copy Markdown
Contributor

One more thing I missed: both commits fail the DCO check. They need a Signed-off-by: line (git commit -s), and the author email is currently a local machine address. Please set git config user.email to a real address and amend or rebase with sign-off, for example git rebase --signoff HEAD~2 then force-push. Also, a correction to my earlier point 2: the AI assistance section is there, but neither box is ticked. Please tick the one that applies.

@Natnaeltewodros Natnaeltewodros changed the title Fix --framework option for darnit run Fix --framework option for darnit run add Fixes #507 to the description. Oct 2, 2026
@Natnaeltewodros Natnaeltewodros changed the title Fix --framework option for darnit run add Fixes #507 to the description. Fix #507 --framework option for darnit run Oct 2, 2026
@mlieberman85

Copy link
Copy Markdown
Contributor

Thanks for picking up #507. darnit run --framework has now landed as part of #556, which removed .baseline.toml; that change needed the option, since .baseline.toml was darnit run's only way to pick a framework. It also covers the points from the earlier review: an unknown framework name exits non-zero, and the tests check that the chosen framework is used. Closing this one as superseded. Thanks again for the contribution.

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.

2 participants