[MAINTENANCE] Restore the GCS datasource tests and run them in CI - #12058
Merged
Conversation
The 14 tests in the pandas and Spark GCS datasource modules patched `...fluent.data_asset.data_connector.google_cloud_storage_data_connector.list_gcs_keys`, but that module moved to `...fluent.data_connector.` The stale path made every patch raise AttributeError, so all 14 failed. Nothing caught it: both modules skip at import when google-cloud-storage is absent, and the only job that installs it selects `-m bigquery`, which matches none of these tests. They were unrunnable everywhere, so the breakage stayed invisible. Correct the patch paths, and give the tests a `gcs_deps` marker with its own requirements file and matrix leg, mirroring how `aws_deps` already covers the S3 datasource tests: SDK installed, no credentials needed. The marker keeps the leg surgical -- it selects only these two modules, so the data connector test modules, which are broken separately by a connector API change, stay dormant rather than turning the new leg red. google-cloud-storage is pinned identically in the bigquery requirements; the new file duplicates that pin rather than pulling in the BigQuery SQL stack for a leg that needs only the storage client.
✅ Deploy Preview for niobium-lead-7998 canceled.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The 14 tests in the pandas and Spark GCS datasource test modules all failed on
develop. They patch:but that module lives at
great_expectations.datasource.fluent.data_connector....— there is nodata_assetsegment. Everymock.patchraisedAttributeError: module '...fluent.data_asset' has no attribute 'data_connector'.Nothing caught it, because these tests could not run anywhere:
if not google.storage: pytest.skip(allow_module_level=True).google-cloud-storageships only inreqs/requirements-dev-bigquery.txt.marker-tests (bigquery)— selects-m bigquery, which matches none of these tests (they wereunit/big).So this PR does two things: corrects the paths, and makes the tests actually run in CI.
Changes
Patch paths corrected in
test_pandas_google_cloud_storage_datasource.pyandtest_spark_google_cloud_storage_datasource.py. This alone takes the 14 tests from failing to passing.New
gcs_depsmarker, mirroring the existingaws_depspattern that already covers the S3 datasource tests — SDK required, no credentials:reqs/requirements-dev-gcs.txtgoogle-cloud-storageonlytasks.pyMARKER_DEPENDENCY_MAP["gcs_deps"]→ that filepyproject.tomltests/conftest.pyREQUIRED_MARKERSci.ymlgcs_depsadded to themarker-testsmatrixThe 16 tests move from
unit/bigtogcs_deps.Why a marker rather than adding the SDK to shared requirements: putting
google-cloud-storageinrequirements-dev-lite.txtwould also un-skiptests/datasource/fluent/data_asset/data_connector/test_google_cloud_storage_data_connector.py, which is broken by a separate, unrelated change —AzureBlobStorageDataConnector.__init__()and its GCS sibling no longer acceptbatching_regex. That would turn CI red. The marker selects only the two modules being fixed here, leaving that one dormant until it is rewritten against the current connector API.The new requirements file duplicates the
google-cloud-storagepin fromrequirements-dev-bigquery.txtrather than reusing that file, so the leg installs the storage client without the BigQuery SQL stack (sqlalchemy-bigquery,pandas_gbq,gcsfs).Testing
pytest -m gcs_depsselects exactly these 16 tests: 14 passed, 2 skipped. The 2 skips are theGOOGLE_APPLICATION_CREDENTIALSgate intest_construct_*_gcs_datasource_without_gcs_options.marker-testswritesgcp-credentials.jsonunconditionally for every leg, so those 2 will execute rather than skip in the new leg. Verified that path works by pointingGOOGLE_APPLICATION_CREDENTIALSat a locally generated throwaway service-account file: all 16 pass, andstorage.Client()construction needs no network.pytest --verify-marker-coverage-and-exitsucceeds (exactly one required marker per test).tests/test_packaging.pypasses — the new requirements file does not disturb the inter-file set assertions.pip install --dry-run -r reqs/requirements-dev-gcs.txt -r reqs/requirements-dev-test.txt -c constraints-dev.txtresolves cleanly.ruff check/ruff formatclean;invoke type-check --ciclean (757 files + stub-source pass).Known remaining work, not in this PR
tests/datasource/fluent/data_asset/data_connector/test_{google_cloud_storage,azure_blob_storage}_data_connector.pycarry the same stale patch path and 14 tests written against the pre-partitioner connector constructor. Correcting only their paths leaves them failing onTypeError: ... unexpected keyword argument 'batching_regex', so they are untouched here; rewriting them against the current API is a separate change. They remain skipped in CI, exactly as before.The new leg cannot be exercised by this PR's CI
ci.ymltriggers onpull_request_target, and GitHub always evaluates that event against the workflow definition on the base branch. Themarker-testsmatrix that ran here isdevelop's, sogcs_depsproduced no job — the 15 legs on this PR are the pre-existing set.Everything the leg does was verified locally instead, by running the exact command the workflow issues:
That covers marker selection, the requirements mapping, and
--up-servicescorrectly no-opping on an empty service list (same asaws_deps). What it cannot cover is GitHub materialising a job from the new matrix entry.Suggestion for whoever merges: route this through the merge queue rather than merging directly. The
merge_groupevent evaluates the workflow from the queue's temporary branch, which does include this change — so thegcs_depsleg will run and gate the merge instead of first appearing ondevelopafter the fact.