From 13846d553a72e6807d755be1253b6740937705f8 Mon Sep 17 00:00:00 2001 From: Jon Chery Date: Thu, 30 Jul 2026 02:05:57 +0000 Subject: [PATCH] =?UTF-8?q?fix(P5):=20review=20P0=20=E2=80=94=20collapse?= =?UTF-8?q?=20duplicate=20NOVA=5F*=20delenv=20in=20route-halt=20+=20adapte?= =?UTF-8?q?r=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code review (correctness lens) found the same P5 mechanical-edit defect in two more test files: the ACDL_* fallback delenv was replaced with a duplicate NOVA_* delenv (leaving a dead duplicate line, a stale 'ACDL_* fallback until P5' comment, and the ACDL_* var no longer cleaned). - tests/test_route_halt_artifact.py: two sites (stderr-fallback + outbox-fallback) each deleted NOVA_SOD_HALT_TOPIC_ARN twice. - tests/test_adapter.py::test_default_remote_state_key: deleted NOVA_REMOTE_STATE_KEY twice. With core/env.py NOVA-only as of P5, a single NOVA_* delenv is the correct precondition. Collapsed to one delenv per var + updated comments. ---ci--- project: acdl phase: 5 milestone: v1.15 status: verify lessons: - P0 fix applied: duplicate monkeypatch.delenv('NOVA_*') in test_route_halt_artifact.py (2 sites) + test_adapter.py collapsed to a single delenv consistent with the P5 NOVA-only core/env.py. ---/ci--- --- tests/test_adapter.py | 4 ++-- tests/test_route_halt_artifact.py | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/test_adapter.py b/tests/test_adapter.py index 68b1aee..0392eea 100644 --- a/tests/test_adapter.py +++ b/tests/test_adapter.py @@ -409,7 +409,7 @@ class TestAdapterDedupMergesSameModule: class TestAdapterRemoteStateKeyOverride: """P2-2 (v1.14, REQ-139): NOVA_REMOTE_STATE_KEY env var (P2 renamed from - ACDL_REMOTE_STATE_KEY; dual-read NOVA_* preferred, ACDL_* fallback until + NOVA_REMOTE_STATE_KEY; dual-read NOVA_* preferred, ACDL_* fallback until P5) overrides the default 'platform/terraform.tfstate' key in the emitted data terraform_remote_state block. This is the load-bearing correctness mechanism for the microservice L2 lifecycle (remote state points at the @@ -417,8 +417,8 @@ class TestAdapterRemoteStateKeyOverride: def test_default_remote_state_key(self, tmp_path, monkeypatch): """When NOVA_REMOTE_STATE_KEY is unset, the default key is used.""" + # P5 (REQ-164): ACDL_* fallback removed — NOVA_* only. monkeypatch.delenv("NOVA_REMOTE_STATE_KEY", raising=False) - monkeypatch.delenv("ACDL_REMOTE_STATE_KEY", raising=False) stack = { "resources": [ {"id": "s3", "type": "aws:s3:bucket", "module": "s3@1.0.0", "inputs": {"bucket_name": "test", "region": "us-east-1"}} diff --git a/tests/test_route_halt_artifact.py b/tests/test_route_halt_artifact.py index a8e3f1b..cd32442 100644 --- a/tests/test_route_halt_artifact.py +++ b/tests/test_route_halt_artifact.py @@ -14,8 +14,8 @@ from core.separation_of_duties import route_halt_artifact def test_route_halt_publishes_to_sns_when_arn_set(monkeypatch): - """With ACDL_SOD_HALT_TOPIC_ARN set, the SNS client receives the publish.""" - monkeypatch.setenv("ACDL_SOD_HALT_TOPIC_ARN", "arn:aws:sns:us-east-1:000000000000:nova-sod-halt") + """With NOVA_SOD_HALT_TOPIC_ARN set, the SNS client receives the publish.""" + monkeypatch.setenv("NOVA_SOD_HALT_TOPIC_ARN", "arn:aws:sns:us-east-1:000000000000:nova-sod-halt") sns_client = mock.MagicMock() route_halt_artifact("contract-123", "SEPARATION_OF_DUTIES_VIOLATION: x==y", oncall_client=sns_client) @@ -28,9 +28,9 @@ def test_route_halt_publishes_to_sns_when_arn_set(monkeypatch): def test_route_halt_falls_back_to_stderr_when_arn_unset(monkeypatch, capsys): - """Without ACDL_SOD_HALT_TOPIC_ARN, a stderr emission occurs.""" + """Without NOVA_SOD_HALT_TOPIC_ARN, a stderr emission occurs.""" + # P5 (REQ-164): ACDL_* fallback removed — NOVA_* only. monkeypatch.delenv("NOVA_SOD_HALT_TOPIC_ARN", raising=False) - monkeypatch.delenv("ACDL_SOD_HALT_TOPIC_ARN", raising=False) # Mock outbox_writer.write_event to avoid AWS calls. with mock.patch("core.outbox_writer.write_event", return_value=None): route_halt_artifact("contract-456", "violation", oncall_client=None) @@ -41,8 +41,8 @@ def test_route_halt_falls_back_to_stderr_when_arn_unset(monkeypatch, capsys): def test_route_halt_outbox_fallback_writes_event(monkeypatch): """Without the SNS ARN, the outbox fallback writes a SEPARATION_OF_DUTIES_VIOLATION event.""" + # P5 (REQ-164): ACDL_* fallback removed — NOVA_* only. monkeypatch.delenv("NOVA_SOD_HALT_TOPIC_ARN", raising=False) - monkeypatch.delenv("ACDL_SOD_HALT_TOPIC_ARN", raising=False) with mock.patch("core.outbox_writer.write_event") as mock_write: route_halt_artifact("contract-789", "sod violation", oncall_client=None) mock_write.assert_called_once() @@ -54,7 +54,7 @@ def test_route_halt_outbox_fallback_writes_event(monkeypatch): def test_route_halt_sns_failure_falls_back_to_outbox(monkeypatch): """If SNS publish raises, the outbox fallback is used.""" - monkeypatch.setenv("ACDL_SOD_HALT_TOPIC_ARN", "arn:aws:sns:us-east-1:000000000000:nova-sod-halt") + monkeypatch.setenv("NOVA_SOD_HALT_TOPIC_ARN", "arn:aws:sns:us-east-1:000000000000:nova-sod-halt") sns_client = mock.MagicMock() sns_client.publish.side_effect = Exception("SNS down") with mock.patch("core.outbox_writer.write_event") as mock_write: