-
Notifications
You must be signed in to change notification settings - Fork 116
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
[SAT-30611] Test Applied Errata report by hostname #17702
base: master
Are you sure you want to change the base?
Conversation
f164549
to
06aa920
Compare
trigger: test-robottelo |
e99cd9b
to
802109c
Compare
802109c
to
3e08e70
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.
One wording nit, but tests LGTM! nice job :)
3e08e70
to
9b11c2a
Compare
trigger: test-robottelo |
9b11c2a
to
5857e78
Compare
PRT Result
|
trigger: test-robottelo |
PRT Result
|
c0fbd09
to
ce930c8
Compare
PRT Result
|
ce930c8
to
42e8db8
Compare
PRT Result
|
42e8db8
to
15c60f0
Compare
PRT Result
|
'feature': 'katello_errata_install', | ||
'inputs': {'errata': str(RHSA_ERRATA)}, | ||
'targeting_type': 'static_query', | ||
'search_query': f'name = {rhsa_host.hostname}', | ||
'organization_id': module_org.id, | ||
}, | ||
)['id'] | ||
# apply RHBA errata to the other host | ||
rhba_task_id = module_target_sat.api.JobInvocation().run( | ||
data={ | ||
'feature': 'katello_errata_install', |
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.
The web UI uses katello_errata_install_by_search
. (katello_errata_install
is from the old UI.)
katello_errata_install_by_search
includes an errata search input. When you select all in the web UI, the errata search input (I think it's called "Errata filter"?) ends up as an empty string.
Before this fix, the magic comment in the rendered REX job template would have resolved to all errata in the system -- and that incorrect info would end up in the Applied Errata report.
After this fix, you should see only the host's applicable errata both in the rendered job template magic comment, and in the report.
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.
Hi Jeremy thanks for the re-review here, pushed a new commit to address these comments.
Please see the katello_errata_install_by_search
REX job template invoked with
empty errata search query (" "
). Right around Ln# 650
I also made dictionaries for the two host's info so loops can be used in each main step , rather than listing out every step twice for each host.
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.
.. the only thing that I have not yet figured out, how to get this 'magic comment' from the rendered job template.
@vsedmik @ColeHiggins2 any ideas?
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.
Not sure what that that 'magic comment' means. Let's find out.
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.
It's saved to the rendered job template here -->
https://github.com/Katello/katello/blob/f3444e8d6584dbaf66d8711cea75b2f52975b296/app/views/foreman/job_templates/install_errata_by_search_query.erb#L20-L21
Should be the exact errata IDs that result from the search query.
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.
you could look for RESOLVED_ERRATA_IDS=
in the template
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.
@JacobCallahan Do you know of a way to read the rendered job template via API or CLI from the JobInvocation
or its task?
Seems the only way we could find was via the Rendered job template preview in UI, but that sounds more like we should build a thorough rendered template preview testcase for UI, as would also need more Airgun work to accept inputs like hostname
and Errata search query
etc, outside of this PRs scope.
If not, I am already checking the task(s) invoked from the job and their status/outputs, would that be sufficient?
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.
@damoore044 unfortunately, that's too component-specific for me to have an answer to. I'd recommend you check the api and see if that exists. If not, then there may be a way in the CLI.
Otherwise, we know some great Devs that might be able to provide a path!
a4e0d38
to
97217e1
Compare
trigger: test-robottelo |
97217e1
to
2b5e19e
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.
Looks good, just one small question, non-blocking.
PRT Result
|
2b5e19e
to
f8c8ba5
Compare
trigger: test-robottelo |
PRT Result
|
f8c8ba5
to
78a6aec
Compare
trigger: test-robottelo |
@chris1984 could you add co-pilot for review on this? thank you |
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.
PR Overview
This PR adds new test coverage for the Hosts - Applied Errata Report Template by hostname and further validates the date fields with an additional test case.
- Introduces a new test "test_positive_applied_errata_for_specific_hosts" to check report generation and errata filtering by hostname.
- Adds a new constant "FAKE_9_YUM_OUTDATED_PACKAGES" to support the test setup for applied errata.
Reviewed Changes
File | Description |
---|---|
tests/foreman/api/test_reporttemplates.py | Adds new test coverage to validate errata reporting by hostname. |
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
assert chost.execute('subscription-manager refresh').status == 0 | ||
assert chost.applicable_errata_count == 0 | ||
assert chost.applicable_package_count == 0 | ||
# job yieled only a single task for one host, success |
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.
There is a spelling error in the comment on line 675; 'yieled' should be corrected to 'yielded'.
# job yieled only a single task for one host, success | |
# job yielded only a single task for one host, success |
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
PRT Result
|
Problem Statement
test_positive_applied_errata_for_specific_hosts
covers SAT-30611 for report filtered by hostname,but also reported was a perceived discrepancy with the date fields, I found we don't yet test the dates for the Hosts - Applied Errata ReportTemplate.
test_positive_applied_errata_by_install_date
now tests the 'Since' and 'Up to' date fields.
_install_date
case in 6.17.z and Stream, will remove that and open in a separate PR so we can merge this bug's coverage. [Robottelo Applied Errata Report by Install Dates: 'Since' and 'Up to' #17713 ]PRT