From 7f0280a2b03a4f7224166dbd3041218bd0fe6c3e Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Mon, 29 Jul 2024 10:29:02 -0700 Subject: [PATCH] Mark some intentional errors as okay This marks several tracebacks as okay; these are generally instances where we are intentionally causing errors in tests. A new default for the okay_tracebacks value is added that matches cases where we log an exception for a git merge conflict. This happens somewhat frequently in tests, intentionally, and should be safe to ignore in all cases. Moreover, this reduces future friction when adding new tests that have git merge conflicts. Change-Id: I35fe020abdfe8337287dbf522b20b3491a1b594a --- tests/base.py | 8 +++++++- tests/unit/test_executor.py | 9 ++++++--- tests/unit/test_gerrit_awskinesis.py | 2 ++ tests/unit/test_gerrit_gcloud_pubsub.py | 2 ++ tests/unit/test_gerrit_kafka.py | 2 ++ tests/unit/test_github_driver.py | 16 +++++++++++++--- tests/unit/test_merger_repo.py | 12 +++++++++++- tests/unit/test_scheduler.py | 11 +++++++---- tests/unit/test_sos.py | 8 +++++++- tests/unit/test_v3.py | 4 ++-- 10 files changed, 59 insertions(+), 15 deletions(-) diff --git a/tests/base.py b/tests/base.py index 2912337b2b..46c05b635e 100644 --- a/tests/base.py +++ b/tests/base.py @@ -2106,10 +2106,16 @@ class TestConfig: def __init__(self, testobj): test_name = testobj.id().split('.')[-1] test = getattr(testobj, test_name) + default_okay_tracebacks = [ + # We log git merge errors at debug level with tracebacks; + # these are typically safe to ignore + 'ERROR: content conflict', + ] self.simple_layout = getattr(test, '__simple_layout__', None) self.gerrit_config = getattr(test, '__gerrit_config__', {}) self.never_capture = getattr(test, '__never_capture__', None) - self.okay_tracebacks = getattr(test, '__okay_tracebacks__', []) + self.okay_tracebacks = getattr(test, '__okay_tracebacks__', + default_okay_tracebacks) self.enable_nodepool = getattr(test, '__enable_nodepool__', False) self.return_data = getattr(test, '__return_data__', []) self.changes = FakeChangeDB() diff --git a/tests/unit/test_executor.py b/tests/unit/test_executor.py index 71974319bd..5d16d1dc2e 100644 --- a/tests/unit/test_executor.py +++ b/tests/unit/test_executor.py @@ -24,12 +24,13 @@ import time from unittest import mock from tests.base import ( - BaseTestCase, - ZuulTestCase, AnsibleZuulTestCase, + BaseTestCase, FIXTURE_DIR, + ZuulTestCase, + iterate_timeout, + okay_tracebacks, simple_layout, - iterate_timeout ) from zuul.executor.sensors.startingbuilds import StartingBuildsSensor @@ -1258,6 +1259,7 @@ class TestExecutorFailure(ZuulTestCase): tenant_config_file = 'config/single-tenant/main.yaml' @mock.patch('zuul.executor.server.ExecutorServer.executeJob') + @okay_tracebacks('Failed to start') def test_executor_job_start_failure(self, execute_job_mock): execute_job_mock.side_effect = Exception('Failed to start') A = self.fake_gerrit.addFakeChange('org/project1', 'master', 'A') @@ -1268,6 +1270,7 @@ class TestExecutorFailure(ZuulTestCase): '- project-merge .* ERROR', A.messages[-1])) + @okay_tracebacks("Transient error") def test_executor_transient_error(self): self.hold_jobs_in_queue = True A = self.fake_gerrit.addFakeChange('org/project1', 'master', 'A') diff --git a/tests/unit/test_gerrit_awskinesis.py b/tests/unit/test_gerrit_awskinesis.py index 8967f25518..7b56b008df 100644 --- a/tests/unit/test_gerrit_awskinesis.py +++ b/tests/unit/test_gerrit_awskinesis.py @@ -23,6 +23,7 @@ import tests.base from tests.base import ( ZuulTestCase, iterate_timeout, + okay_tracebacks, simple_layout, ) @@ -110,6 +111,7 @@ class TestGerritEventSourceAWSKinesis(ZuulTestCase): self.assertEqual(B.reported, 1, "B should be reported") @simple_layout('layouts/simple.yaml') + @okay_tracebacks("invalid literal for int() with base 10: 'nope'") def test_kinesis_bad_checkpoint(self): listener = self.fake_gerrit.event_thread diff --git a/tests/unit/test_gerrit_gcloud_pubsub.py b/tests/unit/test_gerrit_gcloud_pubsub.py index cd069ad483..311daf1242 100644 --- a/tests/unit/test_gerrit_gcloud_pubsub.py +++ b/tests/unit/test_gerrit_gcloud_pubsub.py @@ -25,6 +25,7 @@ import tests.base from tests.base import ( ZuulTestCase, iterate_timeout, + okay_tracebacks, simple_layout, ) @@ -120,6 +121,7 @@ class TestGerritEventSourceGcloudPubsub(ZuulTestCase): super().setUp() @simple_layout('layouts/simple.yaml') + @okay_tracebacks('Test error') def test_gcloud_pubsub(self): A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A') subscriber = self.fake_gerrit.event_thread.subscriber diff --git a/tests/unit/test_gerrit_kafka.py b/tests/unit/test_gerrit_kafka.py index 6ee649b2e5..0fa642f775 100644 --- a/tests/unit/test_gerrit_kafka.py +++ b/tests/unit/test_gerrit_kafka.py @@ -24,6 +24,7 @@ import tests.base from tests.base import ( ZuulTestCase, iterate_timeout, + okay_tracebacks, simple_layout, ) @@ -102,6 +103,7 @@ class TestGerritEventSourceKafka(ZuulTestCase): super().setUp() @simple_layout('layouts/simple.yaml') + @okay_tracebacks('Broker disconnected before response received') def test_kafka(self): A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A') consumer = self.fake_gerrit.event_thread.consumer diff --git a/tests/unit/test_github_driver.py b/tests/unit/test_github_driver.py index 23d1e232a8..a51d4f7247 100644 --- a/tests/unit/test_github_driver.py +++ b/tests/unit/test_github_driver.py @@ -37,9 +37,15 @@ from zuul.model import MergeRequest, EnqueueEvent, DequeueEvent from zuul.zk.change_cache import ChangeKey from tests.util import random_sha1 -from tests.base import (AnsibleZuulTestCase, BaseTestCase, - ZuulGithubAppTestCase, ZuulTestCase, - simple_layout, iterate_timeout) +from tests.base import ( + AnsibleZuulTestCase, + BaseTestCase, + ZuulGithubAppTestCase, + ZuulTestCase, + iterate_timeout, + okay_tracebacks, + simple_layout, +) from tests.base import ZuulWebFixture EMPTY_LAYOUT_STATE = LayoutState("", "", 0, None, {}, -1) @@ -688,6 +694,8 @@ class TestGithubDriver(ZuulTestCase): self.assertIn(expected_warning, A.comments[1]) @simple_layout('layouts/merging-github.yaml', driver='github') + @okay_tracebacks('Unknown merge failure', + 'Method not allowed') def test_report_pull_merge(self): # pipeline merges the pull request on success A = self.fake_github.openFakePullRequest('org/project', 'master', @@ -1458,6 +1466,7 @@ class TestGithubDriver(ZuulTestCase): self.assertEqual(A.merge_message, pr_description) @simple_layout('layouts/basic-github.yaml', driver='github') + @okay_tracebacks('AttributeError') def test_invalid_event(self): # Regression test to make sure the event forwarder thread continues # running in case the event from the GithubEventProcessor is None. @@ -2774,6 +2783,7 @@ class TestGithubDriverEnterpriseCache(ZuulGithubAppTestCase): git.refs.Reference.create(cache, ref, obj, force=True) @simple_layout('layouts/merging-github.yaml', driver='github') + @okay_tracebacks('find remote ref') def test_github_repo_cache(self): # Test that we fetch and configure retries correctly when # using a github enterprise repo cache (the cache can be diff --git a/tests/unit/test_merger_repo.py b/tests/unit/test_merger_repo.py index 2796d8c9d4..5897d6ee18 100644 --- a/tests/unit/test_merger_repo.py +++ b/tests/unit/test_merger_repo.py @@ -27,7 +27,12 @@ from zuul.merger.merger import MergerTree, Repo import zuul.model from zuul.model import MergeRequest from tests.base import ( - BaseTestCase, ZuulTestCase, FIXTURE_DIR, simple_layout, iterate_timeout + BaseTestCase, + FIXTURE_DIR, + ZuulTestCase, + iterate_timeout, + okay_tracebacks, + simple_layout, ) @@ -289,6 +294,7 @@ class TestMergerRepo(ZuulTestCase): self.assertEqual(repo.remotes.origin.refs.missing.commit.hexsha, commit_sha) + @okay_tracebacks('exit code') def test_clone_timeout(self): parent_path = os.path.join(self.upstream_root, 'org/project1') self.patch(git.Git, 'GIT_PYTHON_GIT_EXECUTABLE', @@ -316,6 +322,7 @@ class TestMergerRepo(ZuulTestCase): r'.*exit code\(-9\)'): work_repo.update() + @okay_tracebacks('git_fetch_error.sh') def test_fetch_retry(self): parent_path = os.path.join(self.upstream_root, 'org/project1') self.patch(Repo, 'retry_interval', 1) @@ -364,6 +371,7 @@ class TestMergerRepo(ZuulTestCase): # And now reset the repo. This should not crash work_repo.reset() + @okay_tracebacks('exit code') def test_broken_cache(self): parent_path = os.path.join(self.upstream_root, 'org/project1') work_repo = Repo(parent_path, self.workspace_root, @@ -1339,6 +1347,7 @@ class TestMergerSchemes(ZuulTestCase): self._assertScheme(self.work_root, 'flat') @simple_layout('layouts/overlapping-repos.yaml') + @okay_tracebacks('collides with') def test_golang_collision(self): merger = self._getMerger(scheme=zuul.model.SCHEME_GOLANG) repo = merger.getRepo('gerrit', 'component') @@ -1347,6 +1356,7 @@ class TestMergerSchemes(ZuulTestCase): self.assertIsNone(repo) @simple_layout('layouts/overlapping-repos.yaml') + @okay_tracebacks('collides with') def test_flat_collision(self): merger = self._getMerger(scheme=zuul.model.SCHEME_FLAT) repo = merger.getRepo('gerrit', 'component') diff --git a/tests/unit/test_scheduler.py b/tests/unit/test_scheduler.py index 1ae39fbda7..d5b751d0b1 100644 --- a/tests/unit/test_scheduler.py +++ b/tests/unit/test_scheduler.py @@ -41,14 +41,15 @@ import zuul.merger.merger from zuul.lib import yamlutil as yaml from tests.base import ( + FIXTURE_DIR, + RecordingExecutorServer, SSLZuulTestCase, + TestConnectionRegistry, ZuulTestCase, + iterate_timeout, + okay_tracebacks, repack_repo, simple_layout, - iterate_timeout, - RecordingExecutorServer, - TestConnectionRegistry, - FIXTURE_DIR, skipIfMultiScheduler, ) from zuul.zk.change_cache import ChangeKey @@ -4815,6 +4816,7 @@ class TestScheduler(ZuulTestCase): @simple_layout('layouts/smtp.yaml') @mock.patch('zuul.driver.gerrit.gerritreporter.GerritReporter.report') + @okay_tracebacks('Gerrit failed to report') def test_failed_reporter(self, report_mock): '''Test that one failed reporter doesn't break other reporters''' # Warning hacks. We sort the reports here so that the test is @@ -6568,6 +6570,7 @@ For CI problems and help debugging, contact ci@example.org""" self.assertTrue('YAY' in A.messages[0]) self.assertTrue('BOO' in A.messages[0]) + @okay_tracebacks('git_fail.sh') def test_merge_error(self): # Test we don't get stuck on a merger error self.waitUntilSettled() diff --git a/tests/unit/test_sos.py b/tests/unit/test_sos.py index dd7818686f..eca6bdc715 100644 --- a/tests/unit/test_sos.py +++ b/tests/unit/test_sos.py @@ -19,7 +19,12 @@ from unittest import mock import zuul.model -from tests.base import iterate_timeout, ZuulTestCase, simple_layout +from tests.base import ( + ZuulTestCase, + iterate_timeout, + okay_tracebacks, + simple_layout, +) from zuul.zk.locks import SessionAwareWriteLock, TENANT_LOCK_ROOT from zuul.scheduler import PendingReconfiguration @@ -456,6 +461,7 @@ class TestScaleOutScheduler(ZuulTestCase): dict(name='project-test2', result='SUCCESS', changes='1,1 2,1'), ], ordered=False) + @okay_tracebacks('Unterminated string starting at') def test_pipeline_summary(self): # Test that we can deal with a truncated pipeline summary self.executor_server.hold_jobs_in_build = True diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py index c23f4affcb..0ecd98c2db 100644 --- a/tests/unit/test_v3.py +++ b/tests/unit/test_v3.py @@ -36,10 +36,10 @@ from zuul.zk.blob_store import BlobStore from tests.base import ( AnsibleZuulTestCase, - ZuulTestCase, FIXTURE_DIR, - simple_layout, + ZuulTestCase, iterate_timeout, + simple_layout, skipIfMultiScheduler, ) from tests.util import random_sha1