-
Notifications
You must be signed in to change notification settings - Fork 35
Refactor qmctl test scripts and improve test utilities #894
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
Conversation
Reviewer's GuideThis PR overhauls the qmctl end-to-end test suite by reorganizing and expanding test scripts, standardizing utility functions, and updating the main runner to leverage the new structure. It replaces and introduces dedicated scripts for copy, show, JSON output, and error handling, refactors existing exec/execin tests with robust setup and cleanup, and integrates these into a unified framework. File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @ArtiomDivak - I've reviewed your changes - here's some feedback:
- Consider centralizing the repeated container setup and teardown logic (name, creation, cleanup) into your shared utils to reduce code duplication across exec, execin, and cp test scripts.
- Use dynamically generated container names and temp file paths (e.g. via mktemp) instead of hard-coded “alpine-podman” and /tmp/* to avoid collisions when running tests in parallel.
- Add strict mode flags (set -euo pipefail) at the top of each test script to ensure any unexpected failures or undefined variables cause immediate test failures.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider centralizing the repeated container setup and teardown logic (name, creation, cleanup) into your shared utils to reduce code duplication across exec, execin, and cp test scripts.
- Use dynamically generated container names and temp file paths (e.g. via mktemp) instead of hard-coded “alpine-podman” and /tmp/* to avoid collisions when running tests in parallel.
- Add strict mode flags (set -euo pipefail) at the top of each test script to ensure any unexpected failures or undefined variables cause immediate test failures.
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
c92263a
to
c7954f7
Compare
4b67e9e
to
2aa3ffc
Compare
f8e8cdd
to
50b529c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we are almost there
a58b699
to
0e1c2ca
Compare
15782c2
to
2c2a9b4
Compare
18c6674
to
4b18b4a
Compare
- Replace qmctl_cp.sh with qmctl_cp_bidirectional.sh for better test coverage - Replace qmctl_show.sh with qmctl_show_variants.sh for more comprehensive testing - Add dedicated qmctl_error_handling.sh and qmctl_json_output.sh test scripts - Update existing qmctl_exec.sh and qmctl_execin.sh test scripts - Enhance test utilities in tests/e2e/lib/utils with improved test framework - Update main test runner test_qmctl.sh to use new script structure Addresses issue containers#870 Signed-off-by: Artiom Divak <[email protected]>
Closes #870
Summary by Sourcery
Refactor and expand end-to-end qmctl test suite to improve coverage and test framework support
New Features:
Enhancements:
Tests: