Skip to content

refactor(cli,pytest): improve the cli click interfaces for EEST pytest-based commands #1654

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
May 28, 2025

Conversation

danceratopz
Copy link
Member

@danceratopz danceratopz commented May 26, 2025

🗒️ Description

Refactored CLI wrappers for pytest-based tools (fill, execute, consume) to create a modular, extensible architecture and eliminated the REQUIRED_FLAGS code smell that required dummy parameters for help commands.

The motivation for this refactor is to make the fill CLI more extensible, ready for the two-stage fill execution to enable the "shared-alloc" generation.

🔗 Related Issues

None.

✅ Checklist

  • All: Set appropriate labels for the changes.
  • All: Considered squashing commits to improve commit history.
  • All: Added an entry to CHANGELOG.md.
  • All: Considered updating the online docs in the ./docs/ directory.

@danceratopz danceratopz added type:refactor Type: Refactor scope:consume Scope: Consume command suite scope:fill Scope: fill command scope:execute Scope: Changes to the execute command labels May 26, 2025
@danceratopz danceratopz requested a review from marioevz May 26, 2025 15:48
@danceratopz danceratopz changed the title refactor(cli,pytest): improve the small cli click interfaces used for pytest-based commands refactor(cli,pytest): improve the cli click interfaces for EEST pytest-based commands May 26, 2025
Copy link
Contributor

@spencer-tb spencer-tb left a comment

Choose a reason for hiding this comment

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

LGTM! I really struggled to find anything wrong with the refactor. Played around with all the commands & flags. Its so much cleaner and much easier to maintain now. A really welcome refactor :D

Copy link
Member

@marioevz marioevz left a comment

Choose a reason for hiding this comment

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

LGTM too, ditto on Spencer's comment 🙌

Refactored the pytest-based CLI commands (`fill`, `consume`, `execute`) with a modular architecture that eliminates code duplication and enables future multi-phase execution:

- Created `PytestCommand` base class and `ArgumentProcessor` abstraction
- Added `common_pytest_options` decorator to reduce Click option duplication
- Used `functools.wraps` to properly preserve function metadata in decorators
- Moved argument processing logic into reusable processor classes
- Added verbose logging to show actual pytest commands being executed
- Maintained full backward compatibility of user-facing CLI interface
- Prepared structure for future two-phase fill execution
@danceratopz danceratopz force-pushed the refactor/pytest-cli-interfaces branch from 6979e84 to f2c7dd8 Compare May 28, 2025 05:51
@danceratopz danceratopz merged commit 08f1629 into main May 28, 2025
24 of 25 checks passed
@danceratopz danceratopz deleted the refactor/pytest-cli-interfaces branch May 28, 2025 06:13
@danceratopz
Copy link
Member Author

Weirdly github let me squash merge before this last queued check had ran without bypassing merge rules, so i thought it was a UI glitch.

But the lint workflow failed: it's not a lint fail though, here's the error message: "The hosted runner: GitHub Actions 42 lost communication with the server. Anything in your workflow that terminates the runner process, starves it for CPU/Memory, or blocks its network access can cause this error."

All workflows passed on the squash commit to main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope:consume Scope: Consume command suite scope:execute Scope: Changes to the execute command scope:fill Scope: fill command type:refactor Type: Refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants