From e3d53ee04045f9f3f7245dac1117bfe45ad807f6 Mon Sep 17 00:00:00 2001 From: Giulia Naponiello Date: Sep 11 2018 09:01:46 +0000 Subject: Merge #290 `Always report satisfied requirements` --- diff --git a/functional-tests/consumers/test_resultsdb.py b/functional-tests/consumers/test_resultsdb.py index f85881a..f32d672 100644 --- a/functional-tests/consumers/test_resultsdb.py +++ b/functional-tests/consumers/test_resultsdb.py @@ -52,6 +52,13 @@ def test_consume_new_result( 'policies_satisfied': False, 'decision_context': 'bodhi_update_push_stable', 'product_version': 'fedora-26', + 'satisfied_requirements': [ + { + 'result_id': result['id'], + 'testcase': 'dist.rpmdeplint', + 'type': 'test-result-passed', + }, + ], 'unsatisfied_requirements': [ { 'testcase': 'dist.abicheck', @@ -83,6 +90,7 @@ def test_consume_new_result( 'taskotron_release_critical_tasks'], 'policies_satisfied': False, 'summary': '3 of 3 required test results missing', + 'satisfied_requirements': [], 'unsatisfied_requirements': [ { 'testcase': 'dist.abicheck', @@ -115,6 +123,13 @@ def test_consume_new_result( 'policies_satisfied': True, 'decision_context': 'bodhi_update_push_testing', 'product_version': 'fedora-*', + 'satisfied_requirements': [ + { + 'result_id': result['id'], + 'testcase': 'dist.rpmdeplint', + 'type': 'test-result-passed', + }, + ], 'unsatisfied_requirements': [], 'summary': 'all required tests passed', 'subject': [ @@ -127,6 +142,7 @@ def test_consume_new_result( 'applicable_policies': ['taskotron_release_critical_tasks_for_testing'], 'policies_satisfied': False, 'summary': '1 of 1 required test results missing', + 'satisfied_requirements': [], 'unsatisfied_requirements': [ { 'testcase': 'dist.rpmdeplint', @@ -143,6 +159,13 @@ def test_consume_new_result( 'policies_satisfied': False, 'decision_context': 'bodhi_update_push_stable', 'product_version': 'fedora-26', + 'satisfied_requirements': [ + { + 'result_id': result['id'], + 'testcase': 'dist.rpmdeplint', + 'type': 'test-result-passed', + }, + ], 'unsatisfied_requirements': [ { 'testcase': 'dist.abicheck', @@ -176,6 +199,7 @@ def test_consume_new_result( 'taskotron_release_critical_tasks'], 'policies_satisfied': False, 'summary': '3 of 3 required test results missing', + 'satisfied_requirements': [], 'unsatisfied_requirements': [ { 'testcase': 'dist.abicheck', @@ -208,6 +232,13 @@ def test_consume_new_result( 'policies_satisfied': True, 'decision_context': 'bodhi_update_push_testing', 'product_version': 'fedora-*', + 'satisfied_requirements': [ + { + 'result_id': result['id'], + 'testcase': 'dist.rpmdeplint', + 'type': 'test-result-passed', + }, + ], 'unsatisfied_requirements': [], 'summary': 'all required tests passed', 'subject': [ @@ -222,6 +253,7 @@ def test_consume_new_result( 'applicable_policies': ['taskotron_release_critical_tasks_for_testing'], 'policies_satisfied': False, 'summary': '1 of 1 required test results missing', + 'satisfied_requirements': [], 'unsatisfied_requirements': [ { 'testcase': 'dist.rpmdeplint', @@ -238,51 +270,6 @@ def test_consume_new_result( @mock.patch('greenwave.consumers.resultsdb.fedmsg.config.load_config') @mock.patch('greenwave.consumers.resultsdb.fedmsg.publish') -def test_no_message_for_unchanged_decision( - mock_fedmsg, load_config, requests_session, greenwave_server, - testdatabuilder): - load_config.return_value = {'greenwave_api_url': greenwave_server + 'api/v1.0'} - nvr = testdatabuilder.unique_nvr() - # One result gets the decision in a certain state. - testdatabuilder.create_result(item=nvr, - testcase_name='dist.rpmdeplint', - outcome='PASSED') - # Recording a new version of the same result shouldn't change our decision at all. - new_result = testdatabuilder.create_result( - item=nvr, - testcase_name='dist.rpmdeplint', - outcome='PASSED') - message = { - 'body': { - 'topic': 'resultsdb.result.new', - 'msg': { - 'id': new_result['id'], - 'outcome': 'PASSED', - 'testcase': { - 'name': 'dist.rpmdeplint', - }, - 'data': { - 'item': [nvr], - 'type': ['koji_build'], - } - } - } - } - hub = mock.MagicMock() - hub.config = { - 'environment': 'environment', - 'topic_prefix': 'topic_prefix', - } - handler = resultsdb.ResultsDBHandler(hub) - assert handler.topic == ['topic_prefix.environment.taskotron.result.new'] - handler.consume(message) - # No message should be published as the decision is unchanged since we - # are still missing the required tests. - mock_fedmsg.assert_not_called() - - -@mock.patch('greenwave.consumers.resultsdb.fedmsg.config.load_config') -@mock.patch('greenwave.consumers.resultsdb.fedmsg.publish') def test_invalidate_new_result_with_mocked_cache( mock_fedmsg, load_config, requests_session, greenwave_server, testdatabuilder): @@ -514,6 +501,11 @@ def test_consume_compose_id_result( 'subject_identifier': compose_id, 'summary': '1 of 2 required test results missing', 'previous': old_decision, + 'satisfied_requirements': [{ + 'result_id': result['id'], + 'testcase': 'compose.install_no_user', + 'type': 'test-result-passed' + }], 'unsatisfied_requirements': [{ 'item': {'productmd.compose.id': compose_id}, 'subject_type': 'compose', @@ -524,7 +516,7 @@ def test_consume_compose_id_result( ] } - mock_fedmsg.assert_any_call(topic='decision.update', msg=msg) + mock_fedmsg.assert_called_once_with(topic='decision.update', msg=msg) @mock.patch('greenwave.consumers.resultsdb.fedmsg.config.load_config') @@ -586,6 +578,11 @@ def test_consume_legacy_result( 'policies_satisfied': False, 'decision_context': 'bodhi_update_push_stable', 'product_version': 'fedora-26', + 'satisfied_requirements': [{ + 'result_id': result['id'], + 'testcase': 'dist.rpmdeplint', + 'type': 'test-result-passed' + }], 'unsatisfied_requirements': [ { 'testcase': 'dist.abicheck', @@ -640,6 +637,11 @@ def test_consume_legacy_result( 'policies_satisfied': True, 'decision_context': 'bodhi_update_push_testing', 'product_version': 'fedora-*', + 'satisfied_requirements': [{ + 'result_id': result['id'], + 'testcase': 'dist.rpmdeplint', + 'type': 'test-result-passed' + }], 'unsatisfied_requirements': [], 'summary': 'all required tests passed', 'subject': [ diff --git a/functional-tests/consumers/test_waiverdb.py b/functional-tests/consumers/test_waiverdb.py index 77a0f54..21373c4 100644 --- a/functional-tests/consumers/test_waiverdb.py +++ b/functional-tests/consumers/test_waiverdb.py @@ -20,14 +20,23 @@ def test_consume_new_waiver( nvr = testdatabuilder.unique_nvr() update = testdatabuilder.create_bodhi_update(build_nvrs=[nvr]) updateid = update['updateid'] - result = testdatabuilder.create_result(item=nvr, - testcase_name='dist.abicheck', - outcome='FAILED') + + failing_test = TASKTRON_RELEASE_CRITICAL_TASKS[0] + result = testdatabuilder.create_result( + item=nvr, + testcase_name=failing_test, + outcome='FAILED') + # The rest passed - for testcase_name in TASKTRON_RELEASE_CRITICAL_TASKS[1:]: - testdatabuilder.create_result(item=nvr, - testcase_name=testcase_name, - outcome='PASSED') + passing_tests = TASKTRON_RELEASE_CRITICAL_TASKS[1:] + results = [ + testdatabuilder.create_result( + item=nvr, + testcase_name=testcase_name, + outcome='PASSED') + for testcase_name in passing_tests + ] + testcase = str(result['testcase']['name']) waiver = testdatabuilder.create_waiver(nvr=nvr, testcase_name=testcase, @@ -59,11 +68,23 @@ def test_consume_new_waiver( 'taskotron_release_critical_tasks'], 'policies_satisfied': False, 'summary': '1 of 3 required tests failed', + 'satisfied_requirements': [ + { + 'result_id': results[0]['id'], + 'testcase': passing_tests[0], + 'type': 'test-result-passed' + }, + { + 'result_id': results[1]['id'], + 'testcase': passing_tests[1], + 'type': 'test-result-passed' + } + ], 'unsatisfied_requirements': [ { 'result_id': result['id'], 'item': {'item': nvr, 'type': 'koji_build'}, - 'testcase': 'dist.abicheck', + 'testcase': failing_test, 'type': 'test-result-failed', 'scenario': None, }, @@ -75,6 +96,23 @@ def test_consume_new_waiver( ], 'subject_type': 'koji_build', 'subject_identifier': nvr, + 'satisfied_requirements': [ + { + 'result_id': result['id'], + 'testcase': failing_test, + 'type': 'test-result-passed' + }, + { + 'result_id': results[0]['id'], + 'testcase': passing_tests[0], + 'type': 'test-result-passed' + }, + { + 'result_id': results[1]['id'], + 'testcase': passing_tests[1], + 'type': 'test-result-passed' + } + ], 'unsatisfied_requirements': [], 'summary': 'all required tests passed', 'testcase': testcase, @@ -89,11 +127,23 @@ def test_consume_new_waiver( 'taskotron_release_critical_tasks'], 'policies_satisfied': False, 'summary': '1 of 3 required tests failed', + 'satisfied_requirements': [ + { + 'result_id': results[0]['id'], + 'testcase': passing_tests[0], + 'type': 'test-result-passed' + }, + { + 'result_id': results[1]['id'], + 'testcase': passing_tests[1], + 'type': 'test-result-passed' + } + ], 'unsatisfied_requirements': [ { 'result_id': result['id'], 'item': {'item': nvr, 'type': 'koji_build'}, - 'testcase': 'dist.abicheck', + 'testcase': TASKTRON_RELEASE_CRITICAL_TASKS[0], 'type': 'test-result-failed', 'scenario': None, }, @@ -107,6 +157,23 @@ def test_consume_new_waiver( ], 'subject_type': 'bodhi_update', 'subject_identifier': updateid, + 'satisfied_requirements': [ + { + 'result_id': result['id'], + 'testcase': failing_test, + 'type': 'test-result-passed' + }, + { + 'result_id': results[0]['id'], + 'testcase': passing_tests[0], + 'type': 'test-result-passed' + }, + { + 'result_id': results[1]['id'], + 'testcase': passing_tests[1], + 'type': 'test-result-passed' + } + ], 'unsatisfied_requirements': [], 'summary': 'all required tests passed', 'testcase': testcase, diff --git a/greenwave/api_v1.py b/greenwave/api_v1.py index 95c95e6..4499a8e 100644 --- a/greenwave/api_v1.py +++ b/greenwave/api_v1.py @@ -361,8 +361,10 @@ def make_decision(): 'policies_satisfied': all(answer.is_satisfied for answer in answers), 'summary': summarize_answers(answers), 'applicable_policies': [policy.id for policy in applicable_policies], - 'unsatisfied_requirements': [answer.to_json() for answer in answers - if not answer.is_satisfied], + 'satisfied_requirements': + [answer.to_json() for answer in answers if answer.is_satisfied], + 'unsatisfied_requirements': + [answer.to_json() for answer in answers if not answer.is_satisfied], } if verbose: # Retrieve test results for all items when verbose output is requested. @@ -373,8 +375,6 @@ def make_decision(): res.update({ 'results': results, 'waivers': waivers, - 'satisfied_requirements': - [answer.to_json() for answer in answers if answer.is_satisfied], }) resp = jsonify(res) resp = insert_headers(resp) diff --git a/greenwave/consumers/resultsdb.py b/greenwave/consumers/resultsdb.py index 9e0cbc9..717a1ae 100644 --- a/greenwave/consumers/resultsdb.py +++ b/greenwave/consumers/resultsdb.py @@ -226,4 +226,5 @@ class ResultsDBHandler(fedmsg.consumers.FedmsgConsumer): }) log.debug('Emitted a fedmsg, %r, on the "%s" topic', decision, 'greenwave.decision.update') + print('--------------------', decision['satisfied_requirements']) fedmsg.publish(topic='decision.update', msg=decision)