From 6562bf374ce53a47e6dd125434bd9ddaf6d24adb Mon Sep 17 00:00:00 2001 From: Lukas Holecek Date: Oct 08 2020 12:10:48 +0000 Subject: Cache passed results Store passed test results in cache since it's not expected for them to change to failed once they passed. This avoids doing additional ResultsDB queries. Signed-off-by: Lukas Holecek --- diff --git a/greenwave/resources.py b/greenwave/resources.py index caf7290..ab05cca 100644 --- a/greenwave/resources.py +++ b/greenwave/resources.py @@ -63,6 +63,16 @@ class ResultsRetriever(BaseRetriever): return [result] return [] + # Try to get passing test case result from external cache. + external_cache_key = None + if testcase and not self.since: + external_cache_key = ( + "greenwave.resources:ResultsRetriever|" + f"{subject.type} {subject.identifier} {testcase} {scenarios}") + results = self.get_external_cache(external_cache_key) + if results: + return results + params = { '_distinct_on': 'scenario,system_architecture,system_variant' } @@ -81,6 +91,12 @@ class ResultsRetriever(BaseRetriever): if not testcase: self.cache[cache_key] = results + # Store test case results in external cache if all are passing, + # otherwise retrieve from ResultsDB again later. + if external_cache_key and all( + result.get('outcome') in ('PASSED', 'INFO') for result in results): + self.set_external_cache(external_cache_key, results) + return results def _make_request(self, params, **request_args): @@ -89,6 +105,12 @@ class ResultsRetriever(BaseRetriever): params=params, **request_args) + def get_external_cache(self, key): + return current_app.cache.get(key) + + def set_external_cache(self, key, value): + current_app.cache.set(key, value) + class WaiversRetriever(BaseRetriever): """ diff --git a/greenwave/tests/test_policies.py b/greenwave/tests/test_policies.py index 560d969..d712494 100644 --- a/greenwave/tests/test_policies.py +++ b/greenwave/tests/test_policies.py @@ -44,12 +44,15 @@ class DummyResultsRetriever(ResultsRetriever): self.subject = subject self.testcase = testcase self.outcome = outcome + self.external_cache = {} + self.retrieve_data_called = 0 def _retrieve_data(self, params): + self.retrieve_data_called += 1 if (self.subject and (params.get('item') == self.subject.identifier or params.get('nvr') == self.subject.identifier) and ('type' not in params or self.subject.type in params['type'].split(',')) and - params.get('testcases') == self.testcase): + (params.get('testcases') is None or params.get('testcases') == self.testcase)): return [{ 'id': 123, 'data': { @@ -61,6 +64,12 @@ class DummyResultsRetriever(ResultsRetriever): }] return [] + def get_external_cache(self, key): + return self.external_cache.get(key) + + def set_external_cache(self, key, value): + self.external_cache[key] = value + def test_summarize_answers(): subject = create_test_subject('koji_build', 'nvr') @@ -1442,3 +1451,46 @@ def test_two_rules_no_duplicate(tmpdir): decision = policy.check('fedora-31', subject, results) assert len(decision) == 1 assert isinstance(decision[0], TestResultFailed) + + +def test_cache_all_results_temporarily(): + """ + All results are stored in temporary cache (valid during single request). + """ + subject = create_test_subject('bodhi_update', 'update-1') + results = DummyResultsRetriever(subject, 'sometest', 'FAILED') + + retrieved = results.retrieve(subject, testcase=None) + assert results.retrieve_data_called == 1 + assert retrieved + + cached = results.retrieve(subject, testcase='sometest') + assert results.retrieve_data_called == 1 + assert cached == retrieved + + +def test_cache_passing_results(): + """ + Passing results are stored in external cache because it's not expected that + the outcome changes once they passed. + """ + subject = create_test_subject('bodhi_update', 'update-1') + results = DummyResultsRetriever(subject, 'sometest', 'FAILED') + + retrieved = results.retrieve(subject, testcase=None) + assert results.retrieve_data_called == 1 + assert retrieved + + results2 = DummyResultsRetriever(subject, 'sometest', 'PASSED') + results2.external_cache = results.external_cache + retrieved2 = results2.retrieve(subject, testcase='sometest') + assert results2.retrieve_data_called == 1 + assert retrieved2 + assert retrieved2 != retrieved + + # Result stays PASSED even if the latest is now FAILED. + results3 = DummyResultsRetriever(subject, 'sometest', 'FAILED') + results3.external_cache = results.external_cache + cached = results3.retrieve(subject, testcase='sometest') + assert results3.retrieve_data_called == 0 + assert cached == retrieved2