[6.19.z] Automate setting IoP log levels via satellite-installer - #20818
Conversation
* Automate setting IoP log levels via satellite-installer This PR automates testing of SAT-41750, which introduced satellite-installer options that set the verbosity level of the IoP loggers. It verifies that none of these loggers are set to DEBUG by default, that all of them can be switched to DEBUG using satellite-installer, and that all of them can be switched back to their default values using satellite-installer. It also introduces a helper method for processing satellite-installer help output into a dictionary. * Change Satellite fixture to the correct IoP fixture (cherry picked from commit c484979)
|
Reviewer's GuideAdds an automated CLI test that validates IoP log levels can be inspected, set to DEBUG, and reset to defaults via satellite-installer, along with a helper for parsing installer help output into a dictionary of options and descriptions. 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:
- The
process_iop_log_optionshelper currently splits every non-empty line and uses the first token as an option; consider constraining it to lines that actually look like installer options (e.g., starting with--iop-or matching a regex) to avoid accidentally treating headers or other text as options if the help output format changes. - In
test_set_iop_log_level_via_installer, the installer command to fetch IoP log levels (f'{help_command} | grep iop.*log-level | grep -v reset') is duplicated three times; consider extracting this into a small helper or variable to avoid repetition and reduce the chance of future inconsistencies.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `process_iop_log_options` helper currently splits every non-empty line and uses the first token as an option; consider constraining it to lines that actually look like installer options (e.g., starting with `--iop-` or matching a regex) to avoid accidentally treating headers or other text as options if the help output format changes.
- In `test_set_iop_log_level_via_installer`, the installer command to fetch IoP log levels (`f'{help_command} | grep iop.*log-level | grep -v reset'`) is duplicated three times; consider extracting this into a small helper or variable to avoid repetition and reduce the chance of future inconsistencies.
## Individual Comments
### Comment 1
<location> `tests/foreman/cli/test_rhcloud_iop.py:389` </location>
<code_context>
+ iop_core_engine_log_level_insights_messaging=NEW_LOG_LEVEL,
+ iop_core_engine_log_level_root=NEW_LOG_LEVEL,
+ ).get_command()
+ module_satellite_iop.execute(command)
+
+ # Verify that log levels are now DEBUG
</code_context>
<issue_to_address>
**issue (testing):** Assert the success of `satellite-installer` commands instead of ignoring the exit status.
These `module_satellite_iop.execute(command)` calls for setting/resetting log levels should check the command result. Without asserting `status == 0` (and optionally validating stderr), the test may continue after a failed installer invocation and yield misleading assertion failures later. Please assert on the execution result so the test fails immediately when the installer call itself fails.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| iop_core_engine_log_level_insights_messaging=NEW_LOG_LEVEL, | ||
| iop_core_engine_log_level_root=NEW_LOG_LEVEL, | ||
| ).get_command() | ||
| module_satellite_iop.execute(command) |
There was a problem hiding this comment.
issue (testing): Assert the success of satellite-installer commands instead of ignoring the exit status.
These module_satellite_iop.execute(command) calls for setting/resetting log levels should check the command result. Without asserting status == 0 (and optionally validating stderr), the test may continue after a failed installer invocation and yield misleading assertion failures later. Please assert on the execution result so the test fails immediately when the installer call itself fails.
|
…ab32af9d1722c4536
|
PRT Result |
Cherrypick of PR: #20815
This PR automates testing of SAT-41750, which introduced satellite-installer options that set the verbosity level of the IoP loggers. It verifies that none of these loggers are set to DEBUG by default, that all of them can be switched to DEBUG using satellite-installer, and that all of them can be switched back to their default values using satellite-installer. It also introduces a helper method for processing satellite-installer help output into a dictionary.
Summary by Sourcery
Add automated coverage for configuring IoP log levels via satellite-installer and introduce a helper for parsing installer help output.
Enhancements:
Tests: