Skip to content

Commit

Permalink
Rewrite summarize_answers to be more accurate
Browse files Browse the repository at this point in the history
The summaries are getting pretty inaccurate in certain cases.
Incomplete tests aren't really "missing", but they're treated
as such in the summary. Problems with gating.yaml files are
counted as failed tests, when they really aren't. The special
handling of errors (we kinda count them as both failures and
errors) doesn't really make much sense.

This rewrites summarize_answers entirely to give more accurate
summaries, with logic that's hopefully clearer, and with only
one iteration over the answers. It's fairly similar to the
javascript code in Bodhi which does much the same work (we have
to duplicate this in Bodhi so it can create a single combined
summary for the results of multiple greenwave queries, when it
has to run more than one query).

Signed-off-by: Adam Williamson <awilliam@redhat.com>
  • Loading branch information
AdamWill committed Aug 11, 2023
1 parent 6afad9f commit f01b55f
Show file tree
Hide file tree
Showing 10 changed files with 140 additions and 116 deletions.
4 changes: 2 additions & 2 deletions docs/messaging.rst
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ new test result message from ResultsDB.
"applicable_policies": ["osci_compose_modules"],
"policies_satisfied": false,
"summary": "1 of 2 required test results missing",
"summary": "Of 2 required test(s), 1 result(s) missing",
"satisfied_requirements": [{
"result_id": 7483048,
"testcase": "osci.redhat-module.installability.functional",
Expand All @@ -58,7 +58,7 @@ new test result message from ResultsDB.
"previous": {
"applicable_policies": ["osci_compose_modules"],
"policies_satisfied": false,
"summary": "1 of 2 required tests failed",
"summary": "Of 2 required test(s), 1 test(s) failed",
"satisfied_requirements": [{
"result_id": 7483048,
"testcase": "osci.redhat-module.installability.functional",
Expand Down
2 changes: 1 addition & 1 deletion docs/package-specific-policies.rst
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ For such policy, missing remote rule file could result in the following decision
"applicable_policies": ["some_policy"],
"policies_satisfied": false,
"satisfied_requirements": []
"summary": "1 of 1 required tests failed",
"summary": "Of 1 required test(s), 1 test(s) failed",
"unsatisfied_requirements": [{
"subject_identifier": "nethack-1.2.3-1.f31",
"subject_type": "koji_build",
Expand Down
2 changes: 1 addition & 1 deletion docs/policies.rst
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ decision.
If a remote rule file exists it should contain a policy for each required decision
context. If no tests are required for the particular decision context, there
should be empty rules set, i.e. ``rules: []``. In this case the evaluation result
will be ``no tests are required``. If there is no decision context matching the
will be ``No tests are required``. If there is no decision context matching the
original policy, the result will be ``Cannot find any applicable policies``.

To be able to get remote rule file, Greenwave requires ``REMOTE_RULE_POLICIES``
Expand Down
16 changes: 8 additions & 8 deletions functional-tests/consumers/test_resultsdb.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ def test_consume_new_result(
'source': None,
}
],
'summary': '2 of 3 required test results missing',
'summary': 'Of 3 required test(s), 2 result(s) missing',
'subject': [
{'item': nvr, 'type': 'koji_build'},
],
Expand All @@ -101,7 +101,7 @@ def test_consume_new_result(
'applicable_policies': ['taskotron_release_critical_tasks_with_blocklist',
'taskotron_release_critical_tasks'],
'policies_satisfied': False,
'summary': '3 of 3 required test results missing',
'summary': 'Of 3 required test(s), 3 result(s) missing',
'satisfied_requirements': [],
'unsatisfied_requirements': [
{
Expand Down Expand Up @@ -152,7 +152,7 @@ def test_consume_new_result(
},
],
'unsatisfied_requirements': [],
'summary': 'All required tests passed',
'summary': 'All required tests passed or waived',
'subject': [
{'item': nvr, 'type': 'koji_build'},
],
Expand All @@ -162,7 +162,7 @@ def test_consume_new_result(
'previous': {
'applicable_policies': ['taskotron_release_critical_tasks_for_testing'],
'policies_satisfied': False,
'summary': '1 of 1 required test results missing',
'summary': 'Of 1 required test(s), 1 result(s) missing',
'satisfied_requirements': [],
'unsatisfied_requirements': [
{
Expand Down Expand Up @@ -260,7 +260,7 @@ def test_consume_compose_id_result(
'subject': [{'productmd.compose.id': compose_id}],
'subject_type': 'compose',
'subject_identifier': compose_id,
'summary': '1 of 2 required test results missing',
'summary': 'Of 2 required test(s), 1 result(s) missing',
'previous': old_decision,
'satisfied_requirements': [{
'subject_type': 'compose',
Expand Down Expand Up @@ -380,7 +380,7 @@ def test_consume_legacy_result(
'source': None,
}
],
'summary': '2 of 3 required test results missing',
'summary': 'Of 3 required test(s), 2 result(s) missing',
'subject': [
{
'item': nvr,
Expand Down Expand Up @@ -427,7 +427,7 @@ def test_consume_legacy_result(
'type': 'test-result-passed'
}],
'unsatisfied_requirements': [],
'summary': 'All required tests passed',
'summary': 'All required tests passed or waived',
'subject': [
{
'item': nvr,
Expand Down Expand Up @@ -628,7 +628,7 @@ def test_consume_new_result_container_image(
'subject': [{'item': item_hash, 'type': 'container-image'}],
'subject_type': 'container-image',
'subject_identifier': item_hash,
'summary': 'All required tests passed',
'summary': 'All required tests passed or waived',
'previous': old_decision,
'satisfied_requirements': [{
'subject_type': 'container-image',
Expand Down
4 changes: 2 additions & 2 deletions functional-tests/consumers/test_waiverdb.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ def test_consume_new_waiver(
'applicable_policies': ['taskotron_release_critical_tasks_with_blocklist',
'taskotron_release_critical_tasks'],
'policies_satisfied': False,
'summary': '1 of 3 required tests failed',
'summary': 'Of 3 required test(s), 1 test(s) failed',
'satisfied_requirements': [
{
'subject_type': 'koji_build',
Expand Down Expand Up @@ -160,6 +160,6 @@ def test_consume_new_waiver(
}
],
'unsatisfied_requirements': [],
'summary': 'All required tests passed',
'summary': 'All required tests passed or waived',
'testcase': testcase,
}
42 changes: 21 additions & 21 deletions functional-tests/test_api_v1.py
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ def test_make_a_decision_on_passed_result(requests_session, greenwave_server, te
'taskotron_release_critical_tasks_with_blocklist',
'taskotron_release_critical_tasks',
]
expected_summary = 'All required tests passed'
expected_summary = 'All required tests passed or waived'
assert res_data['summary'] == expected_summary


Expand Down Expand Up @@ -370,7 +370,7 @@ def test_make_a_decision_on_failed_result_with_waiver(
assert res_data['policies_satisfied'] is True
assert 'taskotron_release_critical_tasks' in res_data['applicable_policies']
assert 'taskotron_release_critical_tasks_with_blocklist' in res_data['applicable_policies']
expected_summary = 'All required tests passed'
expected_summary = 'All required tests passed or waived'
assert res_data['summary'] == expected_summary


Expand All @@ -391,7 +391,7 @@ def test_make_a_decision_on_failed_result(requests_session, greenwave_server, te
assert res_data['policies_satisfied'] is False
assert 'taskotron_release_critical_tasks' in res_data['applicable_policies']
assert 'taskotron_release_critical_tasks_with_blocklist' in res_data['applicable_policies']
expected_summary = '1 of 3 required tests failed, 2 results missing'
expected_summary = 'Of 3 required test(s), 1 test(s) failed, 2 result(s) missing'
assert res_data['summary'] == expected_summary
expected_unsatisfied_requirements = [
{
Expand Down Expand Up @@ -437,7 +437,7 @@ def test_make_a_decision_on_queued_result(requests_session, greenwave_server, te
assert res_data['policies_satisfied'] is False
assert 'taskotron_release_critical_tasks' in res_data['applicable_policies']
assert 'taskotron_release_critical_tasks_with_blocklist' in res_data['applicable_policies']
expected_summary = '3 of 3 required test results missing'
expected_summary = 'Of 3 required test(s), 1 test(s) incomplete, 2 result(s) missing'
assert res_data['summary'] == expected_summary
expected_unsatisfied_requirements = [
{
Expand Down Expand Up @@ -483,7 +483,7 @@ def test_make_a_decision_on_running_result(requests_session, greenwave_server, t
assert res_data['policies_satisfied'] is False
assert 'taskotron_release_critical_tasks' in res_data['applicable_policies']
assert 'taskotron_release_critical_tasks_with_blocklist' in res_data['applicable_policies']
expected_summary = '3 of 3 required test results missing'
expected_summary = 'Of 3 required test(s), 1 test(s) incomplete, 2 result(s) missing'
assert res_data['summary'] == expected_summary
expected_unsatisfied_requirements = [
{
Expand Down Expand Up @@ -526,7 +526,7 @@ def test_make_a_decision_on_no_results(requests_session, greenwave_server, testd
assert res_data['policies_satisfied'] is False
assert 'taskotron_release_critical_tasks' in res_data['applicable_policies']
assert 'taskotron_release_critical_tasks_with_blocklist' in res_data['applicable_policies']
expected_summary = '3 of 3 required test results missing'
expected_summary = 'Of 3 required test(s), 3 result(s) missing'
assert res_data['summary'] == expected_summary
expected_unsatisfied_requirements = [
{
Expand Down Expand Up @@ -571,7 +571,7 @@ def test_make_a_decision_on_redhat_cont_image(requests_session, greenwave_server
assert r.status_code == 200
res_data = r.json()
assert res_data['policies_satisfied'] is False
expected_summary = '2 of 2 required tests failed'
expected_summary = 'Of 2 required test(s), 2 test(s) failed'
assert res_data['summary'] == expected_summary
expected_unsatisfied_requirements = [
{
Expand Down Expand Up @@ -623,7 +623,7 @@ def test_subject_type_group(requests_session, greenwave_server, testdatabuilder)
assert res_data['satisfied_requirements'][0]['type'] == 'test-result-passed'
assert res_data['policies_satisfied'] is True

expected_summary = 'All required tests passed'
expected_summary = 'All required tests passed or waived'
assert res_data['summary'] == expected_summary


Expand All @@ -640,7 +640,7 @@ def test_empty_policy_is_always_satisfied(
res_data = r.json()
assert res_data['policies_satisfied'] is True
assert res_data['applicable_policies'] == ['empty-policy']
expected_summary = 'no tests are required'
expected_summary = 'No tests are required'
assert res_data['summary'] == expected_summary
assert res_data['unsatisfied_requirements'] == []

Expand All @@ -664,7 +664,7 @@ def test_bodhi_push_update_stable_policy(
assert res_data['policies_satisfied'] is True
assert 'taskotron_release_critical_tasks' in res_data['applicable_policies']
assert 'taskotron_release_critical_tasks_with_blocklist' in res_data['applicable_policies']
expected_summary = 'All required tests passed'
expected_summary = 'All required tests passed or waived'
assert res_data['summary'] == expected_summary
assert res_data['unsatisfied_requirements'] == []

Expand All @@ -687,7 +687,7 @@ def test_bodhi_nonexistent_bodhi_update_policy(
res_data = r.json()
assert res_data['policies_satisfied'] is True
assert res_data['applicable_policies'] == []
assert res_data['summary'] == 'no tests are required'
assert res_data['summary'] == 'No tests are required'
assert res_data['unsatisfied_requirements'] == []


Expand Down Expand Up @@ -723,7 +723,7 @@ def test_multiple_results_in_a_subject(
assert res_data['policies_satisfied'] is False
assert 'taskotron_release_critical_tasks' in res_data['applicable_policies']
assert 'taskotron_release_critical_tasks_with_blocklist' in res_data['applicable_policies']
assert res_data['summary'] == '1 of 3 required tests failed'
assert res_data['summary'] == 'Of 3 required test(s), 1 test(s) failed'
expected_unsatisfied_requirements = [
{
'item': {'item': nvr, 'type': 'koji_build'},
Expand Down Expand Up @@ -821,7 +821,7 @@ def test_make_a_decision_on_passed_result_with_scenario(
res_data = r.json()
assert res_data['policies_satisfied'] is True
assert res_data['applicable_policies'] == ['openqa_important_stuff_for_rawhide']
expected_summary = 'All required tests passed'
expected_summary = 'All required tests passed or waived'
assert res_data['summary'] == expected_summary


Expand Down Expand Up @@ -859,7 +859,7 @@ def test_make_a_decision_on_failing_result_with_scenario(
res_data = r.json()
assert res_data['policies_satisfied'] is False
assert res_data['applicable_policies'] == ['openqa_important_stuff_for_rawhide']
expected_summary = '1 of 2 required tests failed'
expected_summary = 'Of 2 required test(s), 1 test(s) failed'
assert res_data['summary'] == expected_summary
expected_unsatisfied_requirements = [{
'item': {'productmd.compose.id': compose_id},
Expand Down Expand Up @@ -1049,7 +1049,7 @@ def test_make_a_decision_about_brew_build(requests_session, greenwave_server, te
res_data = r.json()
assert res_data['policies_satisfied'] is True
assert res_data['applicable_policies'] == ['osci_compose']
assert res_data['summary'] == 'All required tests passed'
assert res_data['summary'] == 'All required tests passed or waived'


def test_validate_gating_yaml_valid(requests_session, greenwave_server):
Expand Down Expand Up @@ -1295,7 +1295,7 @@ def test_decision_on_redhat_module(requests_session, greenwave_server, testdatab
assert r.status_code == 200
res_data = r.json()
assert res_data['policies_satisfied'] is True
expected_summary = 'All required tests passed'
expected_summary = 'All required tests passed or waived'
assert res_data['summary'] == expected_summary
assert res_data['results'][0]['data']['type'][0] == 'redhat-module'

Expand All @@ -1319,7 +1319,7 @@ def test_verbose_retrieve_latest_results(requests_session, greenwave_server, tes
assert r.status_code == 200
res_data = r.json()
assert res_data['policies_satisfied'] is True
expected_summary = 'All required tests passed'
expected_summary = 'All required tests passed or waived'
assert res_data['summary'] == expected_summary
assert len(res_data['results']) == 3
for result in res_data['results']:
Expand Down Expand Up @@ -1361,7 +1361,7 @@ def test_make_decision_passed_on_subject_type_bodhi_with_waiver(
assert res_data['applicable_policies'] == [
'bodhi-test-policy',
]
expected_summary = 'All required tests passed'
expected_summary = 'All required tests passed or waived'
assert res_data['summary'] == expected_summary


Expand Down Expand Up @@ -1420,7 +1420,7 @@ def test_verbose_retrieve_latest_results_scenario(requests_session, greenwave_se
assert r.status_code == 200
res_data = r.json()
assert res_data['policies_satisfied'] is True
expected_summary = 'All required tests passed'
expected_summary = 'All required tests passed or waived'
assert res_data['summary'] == expected_summary
assert len(res_data['results']) == 2
for result in res_data['results']:
Expand Down Expand Up @@ -1699,7 +1699,7 @@ def test_make_a_decision_on_passed_result_with_custom_scenario(
assert r.status_code == 200
res_data = r.json()
assert res_data['policies_satisfied'] is True
assert res_data['summary'] == 'All required tests passed'
assert res_data['summary'] == 'All required tests passed or waived'
assert res_data['satisfied_requirements'] == [
{
'subject_identifier': item1_nvr,
Expand Down Expand Up @@ -1766,4 +1766,4 @@ def test_make_a_decision_all_scenarios_waived(
for result in results
]
assert res_data['policies_satisfied'] is True
assert res_data['summary'] == 'All required tests passed'
assert res_data['summary'] == 'All required tests passed or waived'
Loading

0 comments on commit f01b55f

Please sign in to comment.