Test suite: close breaking-change gaps (CI gating, cross-platform runs, contract & log-contract layers)
Mantenedores costumam responder em até 2 dias
Ninguém assumiu esta issue ainda.
Avaliação
- Dificuldade
- 4/5
- Tempo estimado
- 3-5 dias
- Facilidade para iniciantes
- 52/100
- Tipo de issue
- Bug
- Clareza
- Razoavelmente clara
- Status de atividade
- Pouca atividade
- Stack de tecnologia
- docker, github-actions, postgresql, python
- Domínio
- testing-qa, tooling
Direção de pesquisa
Comece em .github/workflows/check_python.yml para inspecionar como detect atualmente controla unit-tests e integration-tests. Em seguida, verifique tests/unit/utils/test_conf_path.py, tests/integration/conftest.py e pyproject.toml em busca das falhas concretas relacionadas a caminhos do Windows e à disponibilidade do Docker, além das lacunas na configuração do pytest. Execute a suíte unitária e, depois, adicione ou verifique os testes de contrato e observabilidade em tests/contract/ e tests/unit/observability/ conforme os arquivos propostos. O trabalho estará concluído quando o CI não ignorar mais alterações de configuração, a coleta de testes de integração for ignorada corretamente sem Docker e os novos testes impuserem a consistência dos contratos de rota, esquema, SQL e autenticação.
Escrita pelo modelo de indexação a partir do texto da issue.
Descrição
Description of Technical Debt
The automated test suite is broad (235 unit tests, 45 integration tests, 94% line coverage on src/) and the integration layer is genuinely good — real Postgres 16 and Kafka via testcontainers, moto for S3/Secrets Manager/EventBridge, an in-process mock JWT provider, and the Lambda handlers invoked through real API Gateway proxy events.
What it does not currently guarantee is the thing we actually want from it: that a breaking change to existing functionality fails CI. There are three structural holes and a set of narrower coverage gaps.
The suite as measured today (local run, master):
| Layer | Tests | Result |
|---|---|---|
Unit (tests/unit/) |
237 | 235 passed, 2 failed on Windows |
Integration (tests/integration/) |
45 | 45 passed in 26 s |
Coverage on src/ |
— | 94% (lowest file: writer_postgres.py at 79%) |
Structural holes
H1 — CI runs no tests at all when non-Python files change.
.github/workflows/check_python.yml gates every job (including unit-tests and integration-tests) behind the detect job, which only looks for changed *.py and requirements*.txt files. A PR that touches only conf/topic_schemas/*.json, conf/access.json, conf/config.json, conf/topic_keys.json, api.yaml, src/**/sql/*.sql, or Dockerfile hits the noop job and merges green.
Those are precisely the highest-risk breaking-change surfaces:
- tightening
requiredin a topic schema → existing producers start getting400 - editing
src/writers/sql/inserts.sqlnamed parameters →WriterPostgresbreaks at runtime - editing
access.json→ silent403for a tenant
H2 — Two unit tests fail on Windows, so make qa cannot pass locally.
tests/unit/utils/test_conf_path.py hardcodes the POSIX separator:
assert conf_dir.endswith("pkg/conf") # line 82
assert conf_dir.endswith("pkg_invalid_current/conf") # line 121
On Windows resolve_conf_dir() returns ...\pkg\conf, so both fail. CI is ubuntu-latest and never sees it. Fix by comparing Path objects or os.path.join("pkg", "conf").
H3 — The integration suite hard-errors instead of skipping when Docker is unavailable.
tests/integration/conftest.py::_prepull_images is scope="session", autouse=True and calls docker.from_env(timeout=300) unconditionally. With no Docker daemon this raises DockerException during collection and the entire suite errors out — make qa gives a stack trace rather than a skip. Contributors without Docker have no usable local QA path.
Coverage gaps
G1 — WriterPostgres._upsert_status_change has no unit test.
writer_postgres.py lines 171–189 (the whole event_type → created_at/started_at/finished_at mapping for JobCreatedEvent, JobCreatedAndStartedEvent, JobStartedEvent, JobFinishedEvent) is exercised only by tests/integration/test_status_change_writer.py. Combined with H1 and H3 that means the newest writer (added in #189) has no regression net on a config-only PR or on a machine without Docker.
G2 — Nothing enforces api.yaml ↔ ROUTE_MAP consistency.
Both currently list the same 8 routes, but they are maintained by hand on both sides. /docs, /stats/{topic_name} and /terminate were each added twice, manually. Drift is undetected until a consumer reads the spec.
G3 — Nothing enforces topic JSON Schema ↔ Postgres writer field access.
_insert_dlchange, _insert_run and _insert_test use direct subscript access for required fields (message["catalog_id"], message["job_ref"], job["status"], …). If a field is dropped from required in the topic schema, validation passes and the writer then raises KeyError → 500. This is a config-only change, so per H1 it also runs no tests.
G4 — The Postgres DDL exists only inside test code.
tests/integration/schemas/postgres_schema.py::SCHEMA_SQL is the only schema definition in the repository; the authoritative production DDL lives elsewhere. Integration tests can pass against a table shape that no longer matches production.
G5 — Nothing verifies the deployable artifact.
The Dockerfile flattens src/, conf/ and api.yaml into ${LAMBDA_TASK_ROOT}. conf_path.py has a dedicated resolution branch (scenario 3) for exactly that layout, but it is only tested against synthetic temp directories. No test builds the image and confirms src.event_gate_lambda.lambda_handler imports and answers /health.
G6 — Coverage gates are inconsistent and far below actual.
- CI:
pytest --cov=. -v tests/unit/ --cov-fail-under=80 Makefile:pytest tests/unit/ --cov=src --cov-fail-under=90- Actual: 94%
The effective ratchet permits a 14-point regression. There is also no per-file floor, so writer_postgres.py at 79% hides behind the aggregate, and no coverage report is published on the PR.
G7 — No pytest configuration.
pyproject.toml has no [tool.pytest.ini_options]: no testpaths, no registered markers (so there is no -m "not integration" escape hatch), and no filterwarnings. The suite already emits, in four integration modules:
PytestRemovedIn10Warning: Class-scoped fixture defined as instance method is deprecated.
That is a silent break waiting for the next pytest major bump.
G8 — Auth negative tests are thin.
Covered: expired token, wrong signing key, missing token, unauthorized sub, case-insensitive sub match. Not covered: alg=none, an HS256 token signed with the RSA public key (algorithm confusion), a token with no sub claim, a non-string sub, and Authorization values with embedded whitespace/newlines. decode_jwt pins algorithms=["RS256"] so these should all be rejected — which is exactly why they deserve regression tests.
G9 — Repository hygiene.
.coverageis tracked in git despite being listed in.gitignore(PR #204 happens to delete it).tests/integration/.tmp_conf/is written bylambda_handler_factoryand only removed if it ends up empty; it is not in.gitignore.
Interaction with PR #204 (structured logging)
#204 (feature/193-improve-service-logging) is well tested for its own surface and should not be blocked by this issue. It adds tests/unit/utils/test_observability.py (13 tests, including correlation-id validation, cold-start flipping and request-scoped-key leakage between warm invocations), tests/unit/utils/test_logging_levels.py, 9 new test_utils.py tests for the rewritten dispatch_request, and a TestCorrelationId class in the integration health tests.
Two things it changes are worth folding into this work rather than leaving implicit:
dispatch_requestnow catches bareExceptionat the boundary instead of a tuple of six exception types. That is the right call, but it means any handler bug now silently becomes a logged500instead of surfacing. The log-contract layer proposed below is what keeps that from becoming a blind spot.HandlerTopic.handle_requestgained JSON-body validation (400for a non-JSON or non-object body) andresolve_request_topicgained a400for a missingtopic_namepath parameter. These are new externally visible status codes thatapi.yamldoes not document — G2 would catch that.
Impact of Technical Debt
- A config-only PR can break production with a fully green CI run. This is the concrete, current risk (H1).
- Contributors on Windows cannot run
make qa(H2), and contributors without Docker get a crash rather than a skip (H3) — both push verification onto CI, which per H1 may not run. - The newest writer (
status_change, #189) has the weakest unit coverage of any module (G1). - Schema/spec/SQL drift is invisible until runtime (G2, G3, G4).
- 94% line coverage overstates confidence: the gate is set at 80, and line coverage says nothing about assertion strength.
Category
Testing / Test Coverage
Priority
Medium - Should be addressed soon
Proposed Solution
Ordered by value per unit of effort.
1. Make CI actually run (H1) — highest value, smallest change
In check_python.yml, split the concerns:
- Keep the
detectgate forpylint-analysis,black-checkandmypy-check(they genuinely only care about*.py). - Run
unit-testsunconditionally on every PR. It takes ~10 s. - Extend the detection glob for
integration-teststo include the behavioural surfaces:
--jq '.[].filename | select(
endswith(".py") or endswith(".sql") or endswith(".yaml") or
(startswith("conf/")) or (startswith("requirements")) or
. == "Dockerfile" or . == "api.yaml"
)'
2. Fix cross-platform and no-Docker execution (H2, H3)
test_conf_path.py: replaceendswith("pkg/conf")withPath(conf_dir) == module_dir / "conf".tests/integration/conftest.py: probe the daemon once and skip cleanly.
@pytest.fixture(scope="session", autouse=True)
def _prepull_images() -> None:
try:
client = docker.from_env(timeout=300)
client.ping()
except docker.errors.DockerException as exc:
pytest.skip(f"Docker is not available, skipping integration tests: {exc}", allow_module_level=True)
...
Add tests/integration/.tmp_conf/ to .gitignore and git rm --cached .coverage.
3. Add a contract test layer — new, fast, no Docker required
A new tests/contract/ package that runs on every PR in well under a second and covers exactly the drift H1 leaves open:
test_api_spec_matches_routes.py— parseapi.yamlpaths, assert set equality againstevent_gate_lambda.ROUTE_MAP∪event_stats_lambda.ROUTE_MAP; assert every status code a handler can return is documented (this immediately catches the new400s from #204).test_schema_matches_writer.py— for each topic, assert every keyWriterPostgresaccesses via subscript is present in the schema'srequiredarray.test_sql_params_match_writer.py— extract%(name)splaceholders fromsrc/writers/sql/inserts.sqlandsrc/readers/sql/stats.sql, assert they equal the dict keys passed by the writer/reader.test_config_consistency.py— assertconstants.TOPIC_*≡ files inconf/topic_schemas/≡ keys inaccess.json, and that everytopic_keys.jsonkey is a known topic and every value is a field defined in that topic's schema.
4. Add a log-contract test layer — protects PR #204's value
Under tests/unit/observability/, drive the handler with caplog/captured stdout and assert:
- every emitted record is valid JSON with
service,level,message,correlation_id,cold_start,resource; - no record ever contains a raw bearer token, a password, or a Secrets Manager payload;
- no request-scoped key (
user,topic) survives from aPOST /topics/{topic}into a followingGET /healthon the same warm container — the integration-level counterpart to the unit test #204 already has; LOG_LEVEL=TRACEproduces redacted, size-capped payloads (safe_serialize_for_logis well unit-tested; this checks the wiring).
5. Close the coverage gaps
- Parametrized unit tests for
_upsert_status_changeover all fourevent_typevalues plus an unknown one, asserting the exactcreated_at/started_at/finished_attriple (G1). - Auth negative tests:
alg=none, HS256-signed-with-public-key, missingsub, non-stringsub(G8). - Align both gates on
--cov=src --cov-fail-under=93, add--cov-report=xmlplus a PR coverage comment, and add a per-file floor so no module drops below ~85% (G6). - Add
[tool.pytest.ini_options]withtestpaths, registeredunit/integration/contractmarkers,--strict-markers, andfilterwarnings = ["error::DeprecationWarning", ...]; fix the class-scoped-fixture deprecation it surfaces (G7).
6. Optional / follow-up layers
- Container smoke job —
docker buildthe Lambda image, run it under the AWS Runtime Interface Emulator, and assert/healthand/topicsrespond. Catches packaging andconf_pathflattening breaks that no unit test can (G5). - Golden payload corpus —
tests/fixtures/payloads/<topic>/*.jsonof historically accepted messages, replayed through validation and the writers. Any schema tightening then fails a test instead of a producer in production. This is the direct answer to "catch breaking changes over existing functionality". - Vendor the authoritative DDL into
db/schema.sqland have the integration fixture load that file instead ofSCHEMA_SQL(G4). - Mutation testing (
mutmut) oversrc/handlers/andsrc/utils/, nightly and non-blocking. 94% line coverage says nothing about assertion strength; this measures it.
Runner and local-environment considerations
- Unit + contract tests: no Docker, no network, run everywhere in ~10 s. Safe to make unconditional on PRs.
- Integration tests: keep
ubuntu-latest; the existing parallel pre-pull with backoff already handles registry flakiness, and the 15-minute timeout is generous against a 26 s local run. Do not addpytest-xdist— the container fixtures are session-scoped and would need reworking first. - After the H2/H3 fixes,
make qapasses on Windows, macOS and Linux, with or without Docker.
Effort Estimate
~3–4 days total: ~0.5 day for H1–H3 and the hygiene fixes, ~1.5 days for the contract and log-contract layers, ~1 day for the coverage gaps and pytest/CI configuration. The container smoke job, golden corpus, DDL vendoring and mutation testing are a separate follow-up of similar size.
Dependencies / Related
- #204 — structured logging via Powertools (in review). Not blocked by this issue; the log-contract layer in step 4 and the
api.yamlstatus-code check in step 3 are the natural follow-ups to it. - #189 — aggregated Postgres writer for the status change topic (the module with the weakest unit coverage, G1).
- #193 — service logging improvements.
- Linguagem predominante
- Python
- Estrelas
- 4
- Forks
- 0
- Merge médio
- 3d 7h
- PRs com merge (30d)
- 11
Preparar o ambiente
- Inclui um Dockerfile ou arquivo Docker Compose
- Tem um modelo de pull request
- Sem guia de contribuição
Primeiros passos
- Leia a issue inteira e depois o guia de contribuição do projeto.
- Comente na issue dizendo que vai assumir — evita que duas pessoas façam o mesmo trabalho.
- Faça um fork do repositório e trabalhe em uma branch.
- Abra um pull request que referencie o número da issue.
Mais de AbsaOSS/EventGate
-
refactoring type:tech-debt
Dificuldade 2/5 1-3 horas Facilidade para iniciantes 84/100
Mantenedores costumam responder em até 2 dias
-
enhancement
Dificuldade 2/5 1-3 horas Facilidade para iniciantes 70/100
Mantenedores costumam responder em até 2 dias
-
bug
Dificuldade 3/5 1-2 dias Facilidade para iniciantes 64/100
Mantenedores costumam responder em até 2 dias
-
Make Writes IdempotentAbertaenhancement
Dificuldade 5/5 Mais de uma semana Facilidade para iniciantes 32/100
Mantenedores costumam responder em até 2 dias
-
infrastructure type:tech-debt
Dificuldade 3/5 1-2 dias Facilidade para iniciantes 70/100
Mantenedores costumam responder em até 2 dias
Todas as issues de AbsaOSS/EventGate
Issues semelhantes
-
Dificuldade 2/5 1-3 horas Facilidade para iniciantes 68/100
-
Clean up dependabot noiseAbertaTask
Dificuldade 2/5 1-3 horas Facilidade para iniciantes 65/100
Mantenedores costumam responder em até 1 dia
-
Dificuldade 2/5 1-3 horas Facilidade para iniciantes 86/100
war-and-code/dircue#200 ·
Mantenedores costumam responder em até 1 dia
-
Dificuldade 2/5 1-3 horas Facilidade para iniciantes 87/100
Mantenedores costumam responder em até 1 dia
-
Dificuldade 2/5 1-3 horas Facilidade para iniciantes 84/100
Mantenedores costumam responder em até 1 dia