Dev-iL commented on code in PR #75642:
URL: https://github.com/apache/airflow/pull/75642#discussion_r4490315999
##########
providers/openlineage/tests/unit/openlineage/extractors/test_manager.py:
##########
@@ -941,32 +941,53 @@ def _hook_exclusion(hook_cls, patterns: list[str]) ->
DatasetFilter:
)
-# gs:// because translating an asset needs the converter of the provider
owning the scheme, and
-# google is a dev dependency of this provider while amazon is not.
-def _collect_gs_parts(collector):
+# file:// because translating an asset needs the converter of the provider
owning the scheme, and
+# common.io is a dev dependency of this provider; other providers' converters
are tested only when installed.
+def _collect_file_parts(collector):
storage_hook, warehouse_hook = _FakeStorageHook(), _FakeWarehouseHook()
- collector.add_output_asset(context=storage_hook,
uri="gs://bucket/staging/part_0")
- collector.add_output_asset(context=storage_hook,
uri="gs://bucket/final/summary")
- collector.add_output_asset(context=warehouse_hook,
uri="gs://bucket/staging/part_1")
+ collector.add_output_asset(context=storage_hook,
uri="file://host/staging/part_0")
+ collector.add_output_asset(context=storage_hook,
uri="file://host/final/summary")
+ collector.add_output_asset(context=warehouse_hook,
uri="file://host/staging/part_1")
def
test_get_hook_lineage_excludes_datasets_per_reporting_hook(hook_lineage_collector):
- _collect_gs_parts(hook_lineage_collector)
+ _collect_file_parts(hook_lineage_collector)
result = ExtractorManager().get_hook_lineage(
- controls=_controls(_hook_exclusion(_FakeStorageHook,
["gs://bucket/staging/.*"]))
+ controls=_controls(_hook_exclusion(_FakeStorageHook,
["file://host/staging/.*"]))
)
assert result == OperatorLineage(
outputs=[
- OpenLineageDataset(namespace="gs://bucket", name="final/summary"),
- OpenLineageDataset(namespace="gs://bucket", name="staging/part_1"),
+ OpenLineageDataset(namespace="file://host", name="/final/summary"),
+ OpenLineageDataset(namespace="file://host",
name="/staging/part_1"),
]
)
[email protected](
+ "scheme",
+ [
+ pytest.param("gs",
marks=skip_if_not_installed("airflow.providers.google"), id="google"),
+ pytest.param("s3",
marks=skip_if_not_installed("airflow.providers.amazon"), id="amazon"),
Review Comment:
That makes sense, but in the current CI setup the test would then never run:
• Regular provider test runs add
`--ignore-glob=providers/*/tests/integration/*` (`run_tests.py`), so
`Providers[openlineage]` skips that folder.
• The `providers-integration-tests` jobs only run for
`TESTABLE_PROVIDERS_INTEGRATIONS`. `openlineage` is only in
`OTHER_PROVIDERS_INTEGRATIONS`, so no job ever collects its integration folder.
That's why it only has empty `__init__.py` files today.
• Those jobs pass `--integration <name>`, which skips any test without a
`pytest.mark.integration(...)` marker. This test needs no external service, so
no marker fits.
Options I see:
1. *Separate unit module* (e.g.
`tests/unit/openlineage/extractors/test_manager_provider_converters.py`). It's
clearly split off from the pure unit tests and still runs. The downside is that
we still call it "unit", inaccurately.
2. *Move it and have breeze run OpenLineage's integration folder* (or, more
generally, unmarked provider integration tests that need no service) in the
regular provider job. This is the right long-term fix, but it changes breeze's
run_tests/selective-checks plus their docs and tests. It deserves its own PR
rather than riding along on a test cleanup.
I'd go with 1 in this PR and open 2 separately. WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
