From c224c36b1287129ad6511634dffead1f6b7b11e2 Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Thu, 16 May 2024 10:28:42 -0700 Subject: [PATCH] Allow early configuration of fake gerrit in tests A followup change is going to require that we set the submitWholeTopic value in our fake gerrit at the start of the test setup sequence (instead of after zuul has already started as we do now). The best way to do that is to add a test decorator so that we can set the value for the test methods that require it. Because we are plumbing an increasing number of what are effectively per-test variables to many different places in the tests, let's centralize these values so that we don't have to pass them all around explicitly. This adds a TestConfig class which holds some test-specific variables, including the global fake change database, and passes this new object around instead of several individual values. There is room for adding more variables in the future and simplifying the class and function signatures of many methods, but this is a start. Change-Id: I514744b1ecca536fd844ed28d29fac3b6ca73a04 --- tests/base.py | 118 ++++++++++++++--------- tests/fakegerrit.py | 4 +- tests/unit/test_circular_dependencies.py | 13 ++- tests/unit/test_component_registry.py | 2 +- tests/unit/test_gerrit.py | 4 +- tests/unit/test_github_driver.py | 2 +- tests/unit/test_gitlab_driver.py | 2 +- tests/unit/test_pagure_driver.py | 4 +- tests/unit/test_scheduler.py | 2 +- tests/unit/test_streaming.py | 8 +- tests/unit/test_web.py | 4 +- tests/unit/test_web_urls.py | 2 +- 12 files changed, 97 insertions(+), 68 deletions(-) diff --git a/tests/base.py b/tests/base.py index 60212dc5cf..92553f51a7 100644 --- a/tests/base.py +++ b/tests/base.py @@ -194,6 +194,20 @@ def never_capture(): return decorator +def gerrit_config(submit_whole_topic=False): + """Configure the fake gerrit + + This allows us to configure the fake gerrit at startup. + """ + + def decorator(test): + test.__gerrit_config__ = dict( + submit_whole_topic=submit_whole_topic, + ) + return test + return decorator + + def registerProjects(source_name, client, config): path = config.get('scheduler', 'tenant_config') with open(os.path.join(FIXTURE_DIR, path)) as f: @@ -243,11 +257,12 @@ class StatException(Exception): class GerritDriverMock(GerritDriver): - def __init__(self, registry, changes, upstream_root, + def __init__(self, registry, test_config, upstream_root, additional_event_queues, poller_events, add_cleanup): super(GerritDriverMock, self).__init__() self.registry = registry - self.changes = changes + self.test_config = test_config + self.changes = test_config.changes self.upstream_root = upstream_root self.additional_event_queues = additional_event_queues self.poller_events = poller_events @@ -259,12 +274,15 @@ class GerritDriverMock(GerritDriver): poll_event = self.poller_events.setdefault(name, threading.Event()) ref_event = self.poller_events.setdefault(name + '-ref', threading.Event()) + submit_whole_topic = self.test_config.gerrit_config.get( + 'submit_whole_topic', False) connection = tests.fakegerrit.FakeGerritConnection( self, name, config, changes_db=db, upstream_root=self.upstream_root, poller_event=poll_event, - ref_watcher_event=ref_event) + ref_watcher_event=ref_event, + submit_whole_topic=submit_whole_topic) if connection.web_server: self.add_cleanup(connection.web_server.stop) @@ -273,11 +291,11 @@ class GerritDriverMock(GerritDriver): class GithubDriverMock(GithubDriver): - def __init__(self, registry, changes, config, upstream_root, + def __init__(self, registry, test_config, config, upstream_root, additional_event_queues, git_url_with_auth): super(GithubDriverMock, self).__init__() self.registry = registry - self.changes = changes + self.changes = test_config.changes self.config = config self.upstream_root = upstream_root self.additional_event_queues = additional_event_queues @@ -298,11 +316,11 @@ class GithubDriverMock(GithubDriver): class PagureDriverMock(PagureDriver): - def __init__(self, registry, changes, upstream_root, + def __init__(self, registry, test_config, upstream_root, additional_event_queues): super(PagureDriverMock, self).__init__() self.registry = registry - self.changes = changes + self.changes = test_config.changes self.upstream_root = upstream_root self.additional_event_queues = additional_event_queues @@ -318,11 +336,11 @@ class PagureDriverMock(PagureDriver): class GitlabDriverMock(GitlabDriver): - def __init__(self, registry, changes, config, upstream_root, + def __init__(self, registry, test_config, config, upstream_root, additional_event_queues): super(GitlabDriverMock, self).__init__() self.registry = registry - self.changes = changes + self.changes = test_config.changes self.config = config self.upstream_root = upstream_root self.additional_event_queues = additional_event_queues @@ -341,19 +359,19 @@ class GitlabDriverMock(GitlabDriver): class TestConnectionRegistry(ConnectionRegistry): - def __init__(self, changes, config, additional_event_queues, - upstream_root, poller_events, git_url_with_auth, - add_cleanup): + def __init__(self, config, test_config, + additional_event_queues, upstream_root, + poller_events, git_url_with_auth, add_cleanup): self.connections = OrderedDict() self.drivers = {} self.registerDriver(ZuulDriver()) self.registerDriver(GerritDriverMock( - self, changes, upstream_root, additional_event_queues, + self, test_config, upstream_root, additional_event_queues, poller_events, add_cleanup)) self.registerDriver(GitDriver()) self.registerDriver(GithubDriverMock( - self, changes, config, upstream_root, additional_event_queues, + self, test_config, config, upstream_root, additional_event_queues, git_url_with_auth)) self.registerDriver(SMTPDriver()) self.registerDriver(TimerDriver()) @@ -362,9 +380,9 @@ class TestConnectionRegistry(ConnectionRegistry): self.registerDriver(NullwrapDriver()) self.registerDriver(MQTTDriver()) self.registerDriver(PagureDriverMock( - self, changes, upstream_root, additional_event_queues)) + self, test_config, upstream_root, additional_event_queues)) self.registerDriver(GitlabDriverMock( - self, changes, config, upstream_root, additional_event_queues)) + self, test_config, config, upstream_root, additional_event_queues)) self.registerDriver(ElasticsearchDriver()) @@ -1458,13 +1476,15 @@ class WebProxyFixture(fixtures.Fixture): class ZuulWebFixture(fixtures.Fixture): - def __init__(self, changes, config, additional_event_queues, - upstream_root, poller_events, git_url_with_auth, - add_cleanup, test_root, info=None): + def __init__(self, config, test_config, + additional_event_queues, upstream_root, + poller_events, git_url_with_auth, add_cleanup, + test_root, info=None): super(ZuulWebFixture, self).__init__() self.config = config self.connections = TestConnectionRegistry( - changes, config, additional_event_queues, upstream_root, + config, test_config, + additional_event_queues, upstream_root, poller_events, git_url_with_auth, add_cleanup) self.connections.configure(config) @@ -1689,15 +1709,11 @@ class BaseTestCase(testtools.TestCase): False) self.addDetail('logging', content) - def shouldNeverCapture(self): - test_name = self.id().split('.')[-1] - test = getattr(self, test_name) - if hasattr(test, '__never_capture__'): - return getattr(test, '__never_capture__') - return False - def setUp(self): super(BaseTestCase, self).setUp() + + self.test_config = TestConfig(self) + self.useFixture(PrometheusFixture()) self.useFixture(GlobalRegistryFixture()) test_timeout = os.environ.get('OS_TEST_TIMEOUT', 0) @@ -1712,7 +1728,7 @@ class BaseTestCase(testtools.TestCase): self.useFixture(fixtures.Timeout(test_timeout, gentle=True)) self.useFixture(fixtures.Timeout(test_timeout + 20, gentle=False)) - if not self.shouldNeverCapture(): + if not self.test_config.never_capture: if (os.environ.get('OS_STDOUT_CAPTURE') == 'True' or os.environ.get('OS_STDOUT_CAPTURE') == '1'): stdout = self.useFixture( @@ -1847,21 +1863,22 @@ class SymLink(object): class SchedulerTestApp: - def __init__(self, log, config, changes, additional_event_queues, + def __init__(self, log, config, test_config, + additional_event_queues, upstream_root, poller_events, git_url_with_auth, add_cleanup, validate_tenants, wait_for_init, disable_pipelines, instance_id): self.log = log self.config = config - self.changes = changes + self.test_config = test_config self.validate_tenants = validate_tenants self.wait_for_init = wait_for_init self.disable_pipelines = disable_pipelines # Register connections from the config using fakes self.connections = TestConnectionRegistry( - self.changes, self.config, + self.test_config, additional_event_queues, upstream_root, poller_events, @@ -1940,7 +1957,7 @@ class SchedulerTestManager: def __init__(self, validate_tenants, wait_for_init, disable_pipelines): self.instances = [] - def create(self, log, config, changes, additional_event_queues, + def create(self, log, config, test_config, additional_event_queues, upstream_root, poller_events, git_url_with_auth, add_cleanup, validate_tenants, wait_for_init, disable_pipelines): @@ -1960,7 +1977,7 @@ class SchedulerTestManager: ) scheduler_config.set("scheduler", "command_socket", command_socket) - app = SchedulerTestApp(log, scheduler_config, changes, + app = SchedulerTestApp(log, scheduler_config, test_config, additional_event_queues, upstream_root, poller_events, git_url_with_auth, add_cleanup, validate_tenants, @@ -2006,6 +2023,22 @@ class SchedulerTestManager: function(instance) +class TestConfig: + def __init__(self, testobj): + test_name = testobj.id().split('.')[-1] + test = getattr(testobj, test_name) + self.simple_layout = None + self.gerrit_config = {} + self.never_capture = None + if hasattr(test, '__simple_layout__'): + self.simple_layout = getattr(test, '__simple_layout__') + if hasattr(test, '__gerrit_config__'): + self.gerrit_config = getattr(test, '__gerrit_config__') + if hasattr(test, '__never_capture__'): + self.gerrit_config = getattr(test, '__never_capture__') + self.changes = FakeChangeDB() + + class ZuulTestCase(BaseTestCase): """A test case with a functioning Zuul. @@ -2198,7 +2231,6 @@ class ZuulTestCase(BaseTestCase): gerritsource.GerritSource.replication_retry_interval = 0.5 gerritconnection.GerritEventConnector.delay = 0.0 - self.changes = FakeChangeDB() if self.load_change_db: self.loadChangeDB() @@ -2223,7 +2255,8 @@ class ZuulTestCase(BaseTestCase): self._configureElasticsearch() executor_connections = TestConnectionRegistry( - self.changes, self.config, self.additional_event_queues, + self.config, self.test_config, + self.additional_event_queues, self.upstream_root, self.poller_events, self.git_url_with_auth, self.addCleanup) executor_connections.configure(self.config, @@ -2260,7 +2293,7 @@ class ZuulTestCase(BaseTestCase): def createScheduler(self): return self.scheds.create( - self.log, self.config, self.changes, + self.log, self.config, self.test_config, self.additional_event_queues, self.upstream_root, self.poller_events, self.git_url_with_auth, self.addCleanup, self.validate_tenants, self.wait_for_init, @@ -2396,18 +2429,15 @@ class ZuulTestCase(BaseTestCase): return config - def setupSimpleLayout(self, config: ConfigParser): + def setupSimpleLayout(self, config): # If the test method has been decorated with a simple_layout, # use that instead of the class tenant_config_file. Set up a # single config-project with the specified layout, and # initialize repos for all of the 'project' entries which # appear in the layout. - test_name = self.id().split('.')[-1] - test = getattr(self, test_name) - if hasattr(test, '__simple_layout__'): - path, driver = getattr(test, '__simple_layout__') - else: + if not self.test_config.simple_layout: return False + path, driver = self.test_config.simple_layout files = {} path = os.path.join(FIXTURE_DIR, path) @@ -3549,11 +3579,11 @@ class ZuulTestCase(BaseTestCase): def saveChangeDB(self): path = os.path.join(self.test_root, "changes.data") - self.changes.save(path) + self.test_config.changes.save(path) def loadChangeDB(self): path = os.path.join(self.test_root, "changes.data") - self.changes.load(path) + self.test_config.changes.load(path) class AnsibleZuulTestCase(ZuulTestCase): diff --git a/tests/fakegerrit.py b/tests/fakegerrit.py index c573fbe777..6be447e394 100644 --- a/tests/fakegerrit.py +++ b/tests/fakegerrit.py @@ -1000,7 +1000,7 @@ class FakeGerritConnection(gerritconnection.GerritConnection): def __init__(self, driver, connection_name, connection_config, changes_db=None, upstream_root=None, poller_event=None, - ref_watcher_event=None): + ref_watcher_event=None, submit_whole_topic=None): if connection_config.get('password'): self.web_server = GerritWebServer(self) @@ -1022,7 +1022,7 @@ class FakeGerritConnection(gerritconnection.GerritConnection): self.fake_checkers = [] self._poller_event = poller_event self._ref_watcher_event = ref_watcher_event - self._fake_submit_whole_topic = False + self._fake_submit_whole_topic = bool(submit_whole_topic) self._fake_submit_permission = True self._fake_project_default_branch = {} self.submit_retry_backoff = 0 diff --git a/tests/unit/test_circular_dependencies.py b/tests/unit/test_circular_dependencies.py index 0fea420d40..0c47aea8c7 100644 --- a/tests/unit/test_circular_dependencies.py +++ b/tests/unit/test_circular_dependencies.py @@ -24,6 +24,7 @@ from zuul.model import PromoteEvent from tests.base import ( iterate_timeout, simple_layout, + gerrit_config, ZuulGithubAppTestCase, ZuulTestCase, ) @@ -3405,8 +3406,8 @@ class TestGerritCircularDependencies(ZuulTestCase): # End check tests + @gerrit_config(submit_whole_topic=True) def test_submitted_together(self): - self.fake_gerrit._fake_submit_whole_topic = True A = self.fake_gerrit.addFakeChange('org/project1', "master", "A", topic='test-topic') B = self.fake_gerrit.addFakeChange('org/project2', "master", "B", @@ -3444,10 +3445,10 @@ class TestGerritCircularDependencies(ZuulTestCase): self.assertEqual(A.data["status"], "MERGED") self.assertEqual(B.data["status"], "MERGED") + @gerrit_config(submit_whole_topic=True) def test_submitted_together_storm(self): # Test that if many changes are uploaded with the same topic, # we handle queries efficiently. - self.fake_gerrit._fake_submit_whole_topic = True self.waitUntilSettled() A = self.fake_gerrit.addFakeChange('org/project', "master", "A", topic='test-topic') @@ -3510,9 +3511,8 @@ class TestGerritCircularDependencies(ZuulTestCase): dict(name="project2-job", changes="2,1 1,1 3,1"), ], ordered=False) + @gerrit_config(submit_whole_topic=True) def test_submitted_together_git(self): - self.fake_gerrit._fake_submit_whole_topic = True - A = self.fake_gerrit.addFakeChange('org/project1', "master", "A") B = self.fake_gerrit.addFakeChange('org/project1', "master", "B") C = self.fake_gerrit.addFakeChange('org/project1', "master", "C") @@ -3547,9 +3547,8 @@ class TestGerritCircularDependencies(ZuulTestCase): changes="1,1 2,1 3,1"), ], ordered=False) + @gerrit_config(submit_whole_topic=True) def test_submitted_together_git_topic(self): - self.fake_gerrit._fake_submit_whole_topic = True - A = self.fake_gerrit.addFakeChange('org/project1', "master", "A", topic='test-topic') B = self.fake_gerrit.addFakeChange('org/project1', "master", "B", @@ -3630,8 +3629,8 @@ class TestGerritCircularDependencies(ZuulTestCase): ], ordered=False) @simple_layout('layouts/submitted-together-per-branch.yaml') + @gerrit_config(submit_whole_topic=True) def test_submitted_together_per_branch(self): - self.fake_gerrit._fake_submit_whole_topic = True self.create_branch('org/project2', 'stable/foo') A = self.fake_gerrit.addFakeChange('org/project1', "master", "A", topic='test-topic') diff --git a/tests/unit/test_component_registry.py b/tests/unit/test_component_registry.py index dfb33338ac..9aac96ae74 100644 --- a/tests/unit/test_component_registry.py +++ b/tests/unit/test_component_registry.py @@ -127,7 +127,7 @@ class TestComponentRegistry(ZuulTestCase): def test_web_component(self): self.useFixture( ZuulWebFixture( - self.changes, self.config, self.additional_event_queues, + self.config, self.test_config, self.additional_event_queues, self.upstream_root, self.poller_events, self.git_url_with_auth, self.addCleanup, self.test_root ) diff --git a/tests/unit/test_gerrit.py b/tests/unit/test_gerrit.py index 080c0aae1e..e2b1aff58e 100644 --- a/tests/unit/test_gerrit.py +++ b/tests/unit/test_gerrit.py @@ -23,6 +23,7 @@ from tests.base import ( AnsibleZuulTestCase, BaseTestCase, simple_layout, + gerrit_config, skipIfMultiScheduler, ZuulTestCase, ) @@ -879,12 +880,11 @@ class TestGerritFake(ZuulTestCase): ret = self.fake_gerrit._getSubmittedTogether(C1, None) self.assertEqual(ret, []) + @gerrit_config(submit_whole_topic=True) def test_submitted_together_whole_topic(self): # Test that the fake submitted together endpoint returns # expected data - # This test verifies behavior with submitWholeTopic=True - self.fake_gerrit._fake_submit_whole_topic = True # A single change A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A') diff --git a/tests/unit/test_github_driver.py b/tests/unit/test_github_driver.py index c5fd99e93d..23d1e232a8 100644 --- a/tests/unit/test_github_driver.py +++ b/tests/unit/test_github_driver.py @@ -2028,7 +2028,7 @@ class TestGithubWebhook(ZuulTestCase): # Start the web server self.web = self.useFixture( - ZuulWebFixture(self.changes, self.config, + ZuulWebFixture(self.config, self.test_config, self.additional_event_queues, self.upstream_root, self.poller_events, self.git_url_with_auth, self.addCleanup, diff --git a/tests/unit/test_gitlab_driver.py b/tests/unit/test_gitlab_driver.py index f7b290e142..49343ff52e 100644 --- a/tests/unit/test_gitlab_driver.py +++ b/tests/unit/test_gitlab_driver.py @@ -44,7 +44,7 @@ class TestGitlabWebhook(ZuulTestCase): # Start the web server self.web = self.useFixture( - ZuulWebFixture(self.changes, self.config, + ZuulWebFixture(self.config, self.test_config, self.additional_event_queues, self.upstream_root, self.poller_events, self.git_url_with_auth, self.addCleanup, diff --git a/tests/unit/test_pagure_driver.py b/tests/unit/test_pagure_driver.py index 1b65949da6..b0ceb6afc3 100644 --- a/tests/unit/test_pagure_driver.py +++ b/tests/unit/test_pagure_driver.py @@ -1007,7 +1007,7 @@ class TestPagureWebhook(ZuulTestCase): super(TestPagureWebhook, self).setUp() # Start the web server self.web = self.useFixture( - ZuulWebFixture(self.changes, self.config, + ZuulWebFixture(self.config, self.test_config, self.additional_event_queues, self.upstream_root, self.poller_events, self.git_url_with_auth, self.addCleanup, @@ -1055,7 +1055,7 @@ class TestPagureWebhookWhitelist(ZuulTestCase): super(TestPagureWebhookWhitelist, self).setUp() # Start the web server self.web = self.useFixture( - ZuulWebFixture(self.changes, self.config, + ZuulWebFixture(self.config, self.test_config, self.additional_event_queues, self.upstream_root, self.poller_events, self.git_url_with_auth, self.addCleanup, diff --git a/tests/unit/test_scheduler.py b/tests/unit/test_scheduler.py index a988188ad9..f9034a1a15 100644 --- a/tests/unit/test_scheduler.py +++ b/tests/unit/test_scheduler.py @@ -97,7 +97,7 @@ class TestSchedulerZone(ZuulTestCase): config.set('executor', 'command_socket', os.path.join(self.test_root, 'executor2.socket')) executor_connections = TestConnectionRegistry( - self.changes, self.config, self.additional_event_queues, + self.config, self.test_config, self.additional_event_queues, self.upstream_root, self.poller_events, self.git_url_with_auth, self.addCleanup) executor_connections.configure(self.config, diff --git a/tests/unit/test_streaming.py b/tests/unit/test_streaming.py index 193020d9b3..3941d5ad07 100644 --- a/tests/unit/test_streaming.py +++ b/tests/unit/test_streaming.py @@ -335,7 +335,7 @@ class TestStreaming(TestStreamingBase): ''' # Start the web server web = self.useFixture( - ZuulWebFixture(self.changes, self.config, + ZuulWebFixture(self.config, self.test_config, self.additional_event_queues, self.upstream_root, self.poller_events, self.git_url_with_auth, self.addCleanup, @@ -418,7 +418,7 @@ class TestStreaming(TestStreamingBase): def test_websocket_streaming(self): # Start the web server web = self.useFixture( - ZuulWebFixture(self.changes, self.config, + ZuulWebFixture(self.config, self.test_config, self.additional_event_queues, self.upstream_root, self.poller_events, self.git_url_with_auth, self.addCleanup, @@ -494,7 +494,7 @@ class TestStreaming(TestStreamingBase): def test_websocket_hangup(self): # Start the web server web = self.useFixture( - ZuulWebFixture(self.changes, self.config, + ZuulWebFixture(self.config, self.test_config, self.additional_event_queues, self.upstream_root, self.poller_events, self.git_url_with_auth, self.addCleanup, @@ -647,7 +647,7 @@ class TestAuthWebsocketStreaming(TestStreamingBase): def test_auth_websocket_streaming(self): # Start the web server web = self.useFixture( - ZuulWebFixture(self.changes, self.config, + ZuulWebFixture(self.config, self.test_config, self.additional_event_queues, self.upstream_root, self.poller_events, self.git_url_with_auth, self.addCleanup, diff --git a/tests/unit/test_web.py b/tests/unit/test_web.py index 31d047669c..77caf94d08 100644 --- a/tests/unit/test_web.py +++ b/tests/unit/test_web.py @@ -55,7 +55,7 @@ class BaseWithWeb(ZuulTestCase): self.zuul_ini_config = FakeConfig(self.config_ini_data) # Start the web server self.web = self.useFixture( - ZuulWebFixture(self.changes, self.config, + ZuulWebFixture(self.config, self.test_config, self.additional_event_queues, self.upstream_root, self.poller_events, self.git_url_with_auth, self.addCleanup, @@ -3744,7 +3744,7 @@ class TestWebStartup(ZuulTestCase): def _start_web(self): # Start the web server self.web = ZuulWebFixture( - self.changes, self.config, self.additional_event_queues, + self.config, self.test_config, self.additional_event_queues, self.upstream_root, self.poller_events, self.git_url_with_auth, self.addCleanup, self.test_root, info=zuul.model.WebInfo.fromConfig(self.zuul_ini_config)) diff --git a/tests/unit/test_web_urls.py b/tests/unit/test_web_urls.py index cc249da396..f7d26e6437 100644 --- a/tests/unit/test_web_urls.py +++ b/tests/unit/test_web_urls.py @@ -27,7 +27,7 @@ class TestWebURLs(ZuulTestCase): def setUp(self): super(TestWebURLs, self).setUp() self.web = self.useFixture( - ZuulWebFixture(self.changes, self.config, + ZuulWebFixture(self.config, self.test_config, self.additional_event_queues, self.upstream_root, self.poller_events, self.git_url_with_auth, self.addCleanup,