Hello everyone,

While working on [1] I ran into a gap in how we categorise tests, and I'd
like to agree on a convention before changing any tooling.

CURRENT SITUATION

We currently have two kinds of tests in each distribution:

- tests/unit - run in the regular CI jobs, including the "lowest
dependencies" runs, where only the providers a distribution declares are
installed.
- tests/integration - documented in
contributing-docs/testing/integration_tests.rst as tests that talk to an
external service running in a Docker container. Each one is marked with
@pytest.mark.integration("<service>") and runs only in that service's
integration job.

There is a third kind with no home: tests that need no service, but do need
another distribution installed. The OpenLineage test from [1] is an
example: it checks that hook-lineage exclusion works on assets translated
by the google and amazon converters. Such a test passes in the regular job
(where everything is installed) and fails in a lowest-dependencies run.

Today these tests sit in tests/unit and opt out of the lowest-dependencies
runs one by one:

- skip_if_force_lowest_dependencies_marker is applied 38 times (14 in
airflow-core, 3 in task-sdk, 4 in providers). Its skip reason is "When
lowest dependencies are set only some providers are loaded".
- Five pytest.importorskip("airflow.providers.<other>") calls in provider
unit tests do the same (fab twice, git, common.ai, modal).
- Others pass only because another provider happens to register something
(a converter, a hook, a connection type). These are invisible until a
lowest-dependencies run breaks.

Moving such a test to tests/integration, as was suggested by Shahar, does
not work today: regular jobs ignore tests/integration entirely, and the
integration jobs skip anything without an integration marker. An unmarked
test in tests/integration is collected by no CI job, and nothing warns
about it.

OPTIONS

1. Broaden "integration". tests/integration becomes "tests that go beyond
their own distribution": an external service, another distribution, or
both. Marked tests keep running in their service job; unmarked tests run in
the regular job and are excluded from lowest-dependencies runs.
   + No new folder, and the name fits the plain-English meaning.
   - Adds meaning to a term that is documented and widely understood as
"needs a Docker service".

2. A new category, e.g. tests/cross_provider (name open for bikeshedding;
"cross_distribution" fits better for airflow-core and task-sdk, which also
have such tests). It runs in the regular job and is excluded from
lowest-dependencies runs.
   + Explicit and visible in the tree, and leaves "integration" untouched.
   - A third test root per distribution, plus plumbing in Breeze, the
folder-checking prek hooks, the mypy folder lists and the pytest path setup.

3. A marker declaring what the test needs, e.g.
@pytest.mark.requires_distributions("apache-airflow-providers-google"),
with tests staying in tests/unit. Lowest-dependencies runs (or any
environment lacking the distribution) skip or deselect them automatically.
   + No files move, and each test states exactly what it needs instead of a
generic skip. This also opens a door for selective checks: a change to
amazon could trigger the tests elsewhere that require amazon, which nothing
does today.
   - Not visible from the directory layout, and it relies on authors adding
the marker (although lowest-dependencies runs already catch omissions, as
they do now).

4. Keep the status quo and only document it: keep
skip_if_force_lowest_dependencies_marker and importorskip, and describe the
pattern in the testing docs.
   + No churn.
   - The skip doesn't say which distribution is missing, and the hidden
cases stay hidden.

What do you think?

Best,
Dev-iL

[1] https://github.com/apache/airflow/pull/75642

Reply via email to