Add foremanctl test for deploy using tuning profiles#21357
Conversation
Reviewer's GuideAdds an automated pytest that verifies File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Consider extracting the
TUNING_PROFILESmapping to a shared fixture or module-level constant so it can be reused by other tuning-related tests and kept in sync with any future profile additions or changes. - The single test currently performs two full
foremanctl deployruns (medium and default) and multiple PostgreSQL checks, which may be slow and harder to debug; consider splitting into two focused tests (apply medium / reset to default) or parametrizing them to keep each scenario smaller and clearer.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider extracting the `TUNING_PROFILES` mapping to a shared fixture or module-level constant so it can be reused by other tuning-related tests and kept in sync with any future profile additions or changes.
- The single test currently performs two full `foremanctl deploy` runs (medium and default) and multiple PostgreSQL checks, which may be slow and harder to debug; consider splitting into two focused tests (apply medium / reset to default) or parametrizing them to keep each scenario smaller and clearer.
## Individual Comments
### Comment 1
<location path="tests/foreman/installer/test_install_foremanctl.py" line_range="207-208" />
<code_context>
+ },
+ }
+ sat = module_sat_ready_rhel
+ # foremanctl deploy with medium tuning profile
+ assert (
+ sat.execute(
+ 'foremanctl deploy --tuning medium',
</code_context>
<issue_to_address>
**nitpick (typo):** Fix the misleading comment for the second deploy call where `--tuning default` is used
The inline comment still says `medium`, but this deploy uses `foremanctl deploy --tuning medium`. Please update the comment to match the actual tuning profile so the test intent stays clear.
Suggested implementation:
```python
# foremanctl deploy with default tuning profile
```
If this comment appears in multiple places (e.g. for both a `--tuning medium` and a `--tuning default` deploy), you should:
1. Apply the above change only to the second deploy call that uses `foremanctl deploy --tuning default`.
2. Leave or adjust the comment for the deploy that uses `--tuning medium` so it still correctly says "medium" there.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
1512117 to
fd91f4b
Compare
fd91f4b to
0f649a7
Compare
|
trigger: test-robottelo |
|
PRT Result |
2318b3f to
826cec5
Compare
|
trigger: test-robottelo |
|
PRT Result |
826cec5 to
9906feb
Compare
|
trigger: test-robottelo |
9906feb to
231fe12
Compare
|
PRT Result |
|
@shubhamsg199 |
I think we should. Asserting the value in the params file seems like the wrong way to verify a change actually happened |
It would be good if we could verify the |
@amolpati30, where are those values saved? In the parameters.yaml file? |
You could try configuring
We do configure PostgreSQL differently starting with |
We can use |
How would that help? |
It provides CPU and memory information, but I am not sure how we can compare it in assertion, since we use the |
Yes, this was my initial thought which I already tried here. But since our default deploy flavor doesn't have the cores needed for medium tuning it resulted failed deploy. Error I got was |
231fe12 to
d194ebb
Compare
|
trigger: test-robottelo |
d194ebb to
28505ac
Compare
|
PRT Result |
1f0f98b to
7592de5
Compare
|
trigger: test-robottelo |
|
PRT Result |
7592de5 to
662f20b
Compare
|
The |
Signed-off-by: Shubham Ganar <sganar@redhat.com>
662f20b to
3634414
Compare
|
trigger: test-robottelo |
|
PRT Result |
|
|
PRT Result |
Adding foremanctl test for deploy using tuning profiles(SAT-41408)
Summary by Sourcery
Add automated coverage for foremanctl deploy behavior when using tuning profiles and resetting to default.
Tests: