Skip to content

Commit 7e48dc8

Browse files
SancusArron Atchisonclaude
committed
Fix CI on the v0.16 JMAP branch: ruff format + offline test setUp (#1205)
* Fix CI on the v0.16 JMAP branch: ruff format + offline test setUp Two independent CI failures on features/628-v016-stalwart-upgrade. 1. `ruff format --check` rejected the commented-out body of `BaseJMAP._debug_dump` (`#with` -> `# with`). Left the dead scaffold in place; only the comment spacing changed. 2. All 10 tests in `test_clients/test_jmap.py` errored in `setUp`. `MailClientAdminJMAP.__init__` calls `_get_user_client`, which builds a real `JMAPClient` and eagerly fetches `/.well-known/jmap` over HTTP. Both test classes constructed the real client before installing `MockJMapClient`, so every test raised ConnectionError -- the `validate-accounts` job only starts postgres/redis/accounts/vite-dev, so there is no Stalwart to reach. Added `build_admin_client()`, which patches `_get_user_client` for the duration of construction to return a `MockJMapClient` (session read from the existing fixture). `TestCreateDkim` no longer needs to reassign `.client` afterwards. Verified in the container with the exact CI commands: `ruff check` clean, `ruff format --check` clean (159 files), and `manage.py test thunderbird_accounts` -> Ran 533 tests, OK. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Fix run-e2e-tests-local: point legacy Stalwart env at renamed service The e2e job's "Wait for local services" gate (`curl --fail http://localhost:8087/health`) returned HTTP 500 until retries were exhausted. Root cause: with the default flag (STALWART_ADMIN_API_USE_JMAP=false) the app uses the legacy client, whose get_telemetry() hits STALWART_BASE_API_URL. That was still http://stalwart:8080, but #1131 renamed the compose service `stalwart` -> `stalwart_legacy` with no network alias, so the hostname no longer resolves. get_telemetry() then raised requests.exceptions.ConnectionError; infra/views.py::__check_stalwart catches only HTTPError (a sibling class), so it escaped and health_check returned 500 instead of its usual 200-with-per-service-status. Fix: point STALWART_BASE_API_URL / STALWART_BASE_JMAP_URL at stalwart_legacy in .env.example and .env.test, and update boot-docker-deps.sh which still started the removed `stalwart` service. Note: this leaves the /health error-handling as-is on purpose. A reachable Stalwart that returns 401/404 raises HTTPError, which __check_stalwart already catches; only an unresolvable host produced the 500. Broadening the except to RequestException would also change prod behaviour (the accounts ECS task would stop self-recycling on a downstream outage, since the ALB /health check would no longer see a 500), so that is left as a separate decision. Verified locally with the full compose stack (postgres, kcpostgres, redis, keycloak, stalwart_legacy, accounts): curl --fail http://localhost:8087/health -> HTTP 200 (all services healthy:true) curl --fail http://localhost:9000/health/ready -> HTTP 200 ruff check / ruff format --check -> clean manage.py test thunderbird_accounts -> Ran 533 tests, OK Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Arron Atchison <aatchison@thunderbird.net> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 4368062 commit 7e48dc8

4 files changed

Lines changed: 24 additions & 8 deletions

File tree

.env.example

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,8 @@ ZENDESK_FORM_BROWSER_FIELD_ID=
8888
ZENDESK_FORM_OS_FIELD_ID=
8989

9090
# These can be the same if they're configured within stalwart as the same port
91-
STALWART_BASE_JMAP_URL=http://stalwart:8081
92-
STALWART_BASE_API_URL=http://stalwart:8080
91+
STALWART_BASE_JMAP_URL=http://stalwart_legacy:8081
92+
STALWART_BASE_API_URL=http://stalwart_legacy:8080
9393
# Bearer token auth (prefixed with api_) or basic auth (username:password base64'd)
9494
# Defaults to admin:accounts
9595
STALWART_API_AUTH_STRING=YWRtaW46YWNjb3VudHM=

.env.test

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,8 @@ ZENDESK_FORM_BROWSER_FIELD_ID=
8282
ZENDESK_FORM_OS_FIELD_ID=
8383

8484
# This URL should not be called without a mock
85-
STALWART_BASE_JMAP_URL=http://stalwart:8081
86-
STALWART_BASE_API_URL=http://stalwart:8080
85+
STALWART_BASE_JMAP_URL=http://stalwart_legacy:8081
86+
STALWART_BASE_API_URL=http://stalwart_legacy:8080
8787
STALWART_API_KEY=this-is-mocked
8888
STALWART_API_AUTH_METHOD=basic
8989

boot-docker-deps.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
#!/bin/sh
22

3-
docker compose up postgres redis kcpostgres keycloak stalwart mailpit
3+
docker compose up postgres redis kcpostgres keycloak stalwart_legacy mailpit

src/thunderbird_accounts/mail/tests/test_clients/test_jmap.py

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,23 @@ def request(self, request_data: JMapRequest, method: Literal['get', 'post'] = 'p
3737
raise NotImplementedError('Monkeypatch me!')
3838

3939

40+
def build_admin_client() -> MailClientAdminJMAP:
41+
"""Build a MailClientAdminJMAP that never touches the network.
42+
43+
``MailClientAdminJMAP.__init__`` builds a real ``JMAPClient`` and eagerly fetches the session
44+
resource over HTTP, so patch ``_get_user_client`` for the duration of construction to hand back
45+
a ``MockJMapClient`` (which reads the session from a fixture) instead.
46+
"""
47+
48+
def _mock_user_client(self, *args, **kwargs) -> MockJMapClient:
49+
client = MockJMapClient('http://stalwart.local', 'admin', 'admin')
50+
client.get_session()
51+
return client
52+
53+
with patch.object(MailClientAdminJMAP, '_get_user_client', _mock_user_client):
54+
return MailClientAdminJMAP()
55+
56+
4057
@override_settings(
4158
STALWART_BASE_API_URL='http://stalwart.test',
4259
STALWART_API_AUTH_STRING='secret',
@@ -48,16 +65,15 @@ def request(self, request_data: JMapRequest, method: Literal['get', 'post'] = 'p
4865
)
4966
class TestCheckDomainDNS(TestMailClientCheckDomainDNS):
5067
def setUp(self):
51-
self.mail_client = MailClientAdminJMAP()
68+
self.mail_client = build_admin_client()
5269
self.domain = 'example.com'
5370
self.expected_host = 'mail.test.com'
5471

5572

5673
class TestCreateDkim(SimpleTestCase):
5774
def setUp(self):
58-
self.mail_client = MailClientAdminJMAP()
75+
self.mail_client = build_admin_client()
5976
self.mail_client.preflight_check = MagicMock()
60-
self.mail_client.client = MockJMapClient('http://stalwart.local', 'admin', 'admin')
6177
self.domain = 'example.com'
6278
self.expected_host = 'mail.test.com'
6379

0 commit comments

Comments
 (0)