From 38969d325d8c95438e696127363802adfade0288 Mon Sep 17 00:00:00 2001 From: Dan Callaghan Date: Jun 20 2017 01:56:59 +0000 Subject: reword summary to be a single line without policy id or item The summary is supposed to be a short, one-line summary of the overall answer. The response still includes the structured "unsatisfied_requirements" key with full details, including policy ids and items. --- diff --git a/functional-tests/test_api_v1.py b/functional-tests/test_api_v1.py index ca32600..c816692 100644 --- a/functional-tests/test_api_v1.py +++ b/functional-tests/test_api_v1.py @@ -145,7 +145,7 @@ def test_make_a_decison_on_passed_result(requests_session, greenwave_server, tes res_data = r.json() assert res_data['policies_satisified'] is True assert res_data['applicable_policies'] == ['1'] - expected_summary = '{}: policy 1 is satisfied as all required tests are passing'.format(nvr) + expected_summary = 'all required tests passed' assert res_data['summary'] == expected_summary @@ -174,7 +174,7 @@ def test_make_a_decison_on_failed_result_with_waiver( res_data = r.json() assert res_data['policies_satisified'] is True assert res_data['applicable_policies'] == ['1'] - expected_summary = '{}: policy 1 is satisfied as all required tests are passing'.format(nvr) + expected_summary = 'all required tests passed' assert res_data['summary'] == expected_summary @@ -195,7 +195,7 @@ def test_make_a_decison_on_failed_result(requests_session, greenwave_server, tes res_data = r.json() assert res_data['policies_satisified'] is False assert res_data['applicable_policies'] == ['1'] - expected_summary = '{}: 1 of 71 required tests failed, the policy 1 is not satisfied'.format(nvr) + expected_summary = '1 of 71 required tests failed' assert res_data['summary'] == expected_summary expected_unsatisfied_requirements = [ { @@ -228,7 +228,7 @@ def test_make_a_decison_on_no_results(requests_session, greenwave_server, testda res_data = r.json() assert res_data['policies_satisified'] is False assert res_data['applicable_policies'] == ['1'] - expected_summary = '{}: no test results found'.format(nvr) + expected_summary = 'no test results found' assert res_data['summary'] == expected_summary expected_unsatisfied_requirements = [ { diff --git a/greenwave/api_v1.py b/greenwave/api_v1.py index 7114047..33a4175 100644 --- a/greenwave/api_v1.py +++ b/greenwave/api_v1.py @@ -46,40 +46,32 @@ def make_decision(): if not applicable_policies: raise NotFound('Cannot find any applicable policies for %s' % product_version) subjects = [item.strip() for item in request.get_json()['subject'] if item] - policies_satisified = True - summary_lines = [] - unsatisfied_requirements = [] + answers = [] timeout = current_app.config['REQUESTS_TIMEOUT'] - for policy in applicable_policies: - for item in subjects: - # XXX make this more efficient than just fetching everything + for item in subjects: + # XXX make this more efficient than just fetching everything + response = requests_session.get( + current_app.config['RESULTSDB_API_URL'] + '/results', + params={'item': item, 'limit': '1000'}, timeout=timeout) + response.raise_for_status() + results = response.json()['data'] + if results: response = requests_session.get( - current_app.config['RESULTSDB_API_URL'] + '/results', - params={'item': item, 'limit': '1000'}, timeout=timeout) + current_app.config['WAIVERDB_API_URL'] + '/waivers/', + params={'product_version': product_version, + 'result_id': ','.join(str(result['id']) for result in results)}, + timeout=timeout) response.raise_for_status() - results = response.json()['data'] - if results: - response = requests_session.get( - current_app.config['WAIVERDB_API_URL'] + '/waivers/', - params={'product_version': product_version, - 'result_id': ','.join(str(result['id']) for result in results)}, - timeout=timeout) - response.raise_for_status() - waivers = response.json()['data'] - else: - waivers = [] - - answers = policy.check(item, results, waivers) - if not all(answer.is_satisfied for answer in answers): - policies_satisified = False - summary_lines.append('{}: {}'.format(item, summarize_answers(answers, policy.id))) - unsatisfied_requirements.extend(answer for answer in answers - if not answer.is_satisfied) - + waivers = response.json()['data'] + else: + waivers = [] + for policy in applicable_policies: + answers.extend(policy.check(item, results, waivers)) res = { - 'policies_satisified': policies_satisified, - 'summary': '\n'.join(summary_lines), + 'policies_satisified': all(answer.is_satisfied for answer in answers), + 'summary': summarize_answers(answers), 'applicable_policies': [policy.id for policy in applicable_policies], - 'unsatisfied_requirements': [a.to_json() for a in unsatisfied_requirements], + 'unsatisfied_requirements': [answer.to_json() for answer in answers + if not answer.is_satisfied], } return jsonify(res), 200 diff --git a/greenwave/policies.py b/greenwave/policies.py index 91c7388..ebb9c52 100644 --- a/greenwave/policies.py +++ b/greenwave/policies.py @@ -77,7 +77,7 @@ class TestResultFailed(RuleNotSatisfied): } -def summarize_answers(answers, policy_id): +def summarize_answers(answers): """ Produces a one-sentence human-readable summary of the result of evaluating a policy. @@ -88,11 +88,10 @@ def summarize_answers(answers, policy_id): str: Human-readable summary. """ if all(answer.is_satisfied for answer in answers): - return 'policy {} is satisfied as all required tests are passing'.format(policy_id) + return 'all required tests passed' failure_count = len([answer for answer in answers if isinstance(answer, TestResultFailed)]) if failure_count: - return ('{} of {} required tests failed, the policy {} is not satisfied'.format( - failure_count, len(answers), policy_id)) + return ('{} of {} required tests failed'.format(failure_count, len(answers))) if all(isinstance(answer, TestResultMissing) for answer in answers): return 'no test results found' # XXX need to handle some missing but others passing diff --git a/greenwave/tests/test_policies.py b/greenwave/tests/test_policies.py index d997c18..3f7d953 100644 --- a/greenwave/tests/test_policies.py +++ b/greenwave/tests/test_policies.py @@ -5,15 +5,15 @@ from greenwave.policies import summarize_answers, RuleSatisfied, TestResultMissi def test_summarize_answers(): - assert summarize_answers([RuleSatisfied()], '1') == \ - 'policy 1 is satisfied as all required tests are passing' - assert summarize_answers([TestResultFailed('item', 'test', 'id'), RuleSatisfied()], '1') == \ - '1 of 2 required tests failed, the policy 1 is not satisfied' - assert summarize_answers([TestResultMissing('item', 'test')], '1') == \ + assert summarize_answers([RuleSatisfied()]) == \ + 'all required tests passed' + assert summarize_answers([TestResultFailed('item', 'test', 'id'), RuleSatisfied()]) == \ + '1 of 2 required tests failed' + assert summarize_answers([TestResultMissing('item', 'test')]) == \ 'no test results found' assert summarize_answers([TestResultMissing('item', 'test'), - TestResultFailed('item', 'test', 'id')], '1') == \ - '1 of 2 required tests failed, the policy 1 is not satisfied' + TestResultFailed('item', 'test', 'id')]) == \ + '1 of 2 required tests failed' # XXX fix this one - assert summarize_answers([TestResultMissing('item', 'test'), RuleSatisfied()], '1') == \ + assert summarize_answers([TestResultMissing('item', 'test'), RuleSatisfied()]) == \ 'inexplicable result'