Skip to content

Add e2e tests and its CI - #210

Open
f-necas wants to merge 12 commits into
mainfrom
test/e2e-local-files
Open

f-necas wants to merge 12 commits into
mainfrom
test/e2e-local-files

Conversation

@f-necas

@f-necas f-necas commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Needs #221

@f-necas
f-necas force-pushed the test/e2e-local-files branch from fbaf8a4 to 97e0c1d Compare October 26, 2026 11:05
@f-necas
f-necas marked this pull request as draft October 27, 2026 08:21
@f-necas f-necas changed the title feat: vibe coded e2e tests Add e2e tests and its CI Oct 27, 2026
@f-necas
f-necas marked this pull request as ready for review October 27, 2026 18:58
@f-necas

f-necas commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor Author

@LHBruneton-C2C Green !

API's and urls are now mocked up and we do not use external services.

@tonio tonio left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this, removing the dependency on external hosts (local file server + the stack's geor:public_layer) is a big improvement over what's on main, and the CI is easy to debug (direct then gateway health checks, logs and traces uploaded only on failure). The encoding cases (misnamed .cst, Latin-9 œ) are exactly the kind of regression e2e tests should catch.

Some points before merging:

1. Binary test data in git history. e2e-tests/files/donnees-2023-reg02.parquet is 38 MB (about 46 MB for all fixtures). Deleting it later won't remove it from history. It reads more like a load test than a functional one, and a-epci2025.parquet already covers parquet. Could we drop it, or generate it in a fixture, or download it only in a nightly job? Could you also add the source and license of the datasets (MEL, data.gouv.fr) to the README?

2. Scope. This PR also contains two functional changes unrelated to tests:

  • run_in_threadpool in staging.py: a good fix (it also helps with the Airflow executor, which does a blocking HTTP call inside an async def handler), but it deserves its own PR or commit.
  • Rejecting multi-layer GeoPackages: a user-facing behavior change that should be agreed on separately (see the inline comment).

Could you split these into separate commits or PRs and squash the rest? "vibe coded e2e tests" and the two "add e2e ci" commits shouldn't land as-is. The PR description is also empty.

3. CI cost. The job takes about 15 min and runs on every PR to main, docs and Dependabot bumps included. Please add a paths: filter (apps/**, libs/**, e2e-tests/**, docker/**, the workflow file), or run it on a label, nightly or on workflow_dispatch.

4. Tooling. apps/frontend/ still has a Cypress setup (4 npm deps, test:e2e* scripts, a commented-out job in frontend-checks.yml) whose only test checks a "Lorem ipsum" placeholder. With this PR, Playwright (Python) becomes the de facto e2e framework. I think that's the right choice, since these tests mostly check pipeline output (WFS hits, coordinates, encodings) and fit the Python/uv/pytest stack. We should remove the dead Cypress setup in a follow-up and record the decision in AGENTS.md, so we don't end up with two frameworks.

5. Coverage that silently doesn't run in CI. The latest run shows 2 skipped:

  • test_import_csv_lat_lon_2154 is skipped because the default PROJECTIONS has no EPSG:2154.
  • The credential tests are skipped because the secrets are not set (and are never available for fork PRs anyway).

Please fix the first (inline comment), and state explicitly that FTP and Basic Auth are not covered in CI.

Comment thread .github/workflows/e2e-tests.yml Resolved
Comment thread .github/workflows/e2e-tests.yml Resolved
Comment thread .github/workflows/e2e-tests.yml Resolved
Comment thread .github/workflows/e2e-tests.yml Resolved
Comment thread e2e-tests/tests/common.py Resolved
Comment thread e2e-tests/tests/common.py Resolved
Comment thread e2e-tests/tests/test_import.py Resolved
@f-necas
f-necas force-pushed the test/e2e-local-files branch from e7761cc to 8345b2a Compare October 30, 2026 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants