awslabs / awslabs/synthetically_engineered_evaluation_data

CI: enforce the existing PR checks as required, and close the coverage gaps

Open
#19 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
9
Forks
1
PR merge metrics
No merged PRs in 30d

Description

## Context

PR-triggered automation already exists — this issue is about **enforcement and coverage gaps**, not adding CI from scratch.

Current state on `main` (`.github/workflows/test.yml`, `on: pull_request`):

- `pytest` — full unit suite
- `build-smoke` — builds the wheel, installs into a clean venv, imports, runs `seed-data --help`
- CodeQL (`Analyze (python)`, `Analyze (actions)`) via default setup

PR #15 additionally introduces two jobs:

- `lint` — `ruff check .` against a pinned ruff (`>=0.15,<0.16`)
- `test-base` — builds the wheel, installs with **no extras**, asserts `pandas` is absent, runs the suite (structured tests self-skip via `importorskip`)

## Gap 1 — new checks aren't required (primary)

`main`'s branch protection lists only:

```
required_status_checks.contexts = ["build-smoke", "pytest"]
```

So `lint` and `test-base` run and report, but a PR can merge with either one red. `test-base` is the only thing guarding the `[structured]` optional-dependency boundary that keeps `pip install seed-data` lean — an unguarded regression there silently ships pandas to document-only users.

**Action:** after #15 merges, add `lint` and `test-base` to the required contexts.

## Gap 2 — zero required approving reviews

`required_approving_review_count: 0` and `require_code_owner_reviews: false`, so a PR can merge with no human review. Worth raising to 1 for a public `awslabs` repo.

## Gap 3 — single Python version

`requires-python = ">=3.12"` but CI only exercises 3.12. Anything ">=3.12" claims support for 3.13+ too, and 3.13 is where the scientific stack most often drifts. Either matrix `pytest` over 3.12/3.13 or narrow the declared range to match what's tested.

## Gap 4 — integration tests never run in CI

`tests/integration/` is excluded by `addopts = --ignore=tests/integration` and needs live Bedrock. It's the only coverage of the real generation path. Consider a manually-dispatched (`workflow_dispatch`) or scheduled job with OIDC credentials, kept off the per-PR path so PRs stay fast and don't require secrets from forks.

## Gap 5 — no coverage reporting

No `pytest-cov` or coverage gate, so coverage can regress unnoticed. Lowest priority of these.

## Suggested order

1. Require `lint` + `test-base` (config-only, no code)
2. Require 1 approving review (config-only)
3. Python version matrix, or narrow `requires-python`
4. Opt-in integration job
5. Coverage reporting

Items 1–2 are repo-settings changes with no code; 3–5 need workflow edits.

Contributor guide

Open the contributing guide

Research direction

Review .github/workflows/test.yml, pyproject.toml for addopts and requires-python, the current branch-protection settings, and PR #15's lint and test-base jobs. Start by comparing reported checks with required contexts; done means the agreed enforcement, version, integration, and coverage changes are implemented and their CI or repository settings are verified.

Written by the indexing model from the issue text.

Assessment

Tech stack
github-actions, python
Domain
ci-cd, devops
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.