Skip to content

Remove the dead cloudant_fake module, or add its missing tests #72269

Description

@jroachgolf84

Description

Module Expected test file
providers/cloudant/src/airflow/providers/cloudant/cloudant_fake.py providers/cloudant/tests/unit/cloudant/test_cloudant_fake.py

It is currently suppressed in the OVERLOOKED_TESTS allowlist in airflow-core/tests/unit/always/test_project_structure.py.

This is a scoped subset of the meta issue #35442, limited to the cloudant provider.

What should these tests cover?

Probably nothing, and that is the finding worth recording here. cloudant_fake.py defines two no-op stand-ins, CloudantV1 and CouchDbSessionAuthenticator, whose own docstrings describe them as a "Phony class to pass mypy when real class is not imported". Every method body is empty, so there is no behaviour a test could meaningfully assert.

More to the point, the module is dead. It existed to serve one conditional import in the hook:

if sys.version_info < (3, 10):
    from airflow.providers.cloudant.cloudant_fake import CloudantV1, CouchDbSessionAuthenticator
else:
    from ibmcloudant import CloudantV1, CouchDbSessionAuthenticator

That fallback was needed while ibmcloudant was excluded on Python 3.9, as recorded in the excluded-python-versions block that used to sit in providers/cloudant/provider.yaml. #52072 dropped Python 3.9 support, removed the version guard, and left the hook importing CloudantV1 and CouchDbSessionAuthenticator directly from ibmcloudant. The stub module was not removed with it.

A search across the repository now finds no import of airflow.providers.cloudant.cloudant_fake in any source, test, or configuration file. Its only remaining reference anywhere is its own entry in OVERLOOKED_TESTS.

So the sensible resolution is removal rather than new tests:

  1. Delete providers/cloudant/src/airflow/providers/cloudant/cloudant_fake.py and drop its OVERLOOKED_TESTS entry.
  2. If it turns out the module is deliberately retained for a mypy path that is not visible from the source, keep it and add a minimal test_cloudant_fake.py asserting both classes are constructible and that set_service_url accepts a URL, then drop the allowlist entry either way.

Option 1 looks correct given the history above, but it is worth a second opinion from anyone with context on the original optional-import arrangement before the file is deleted.

Definition of Done

  1. Confirm whether cloudant_fake.py is still needed by anything.
  2. Either delete the module, or add providers/cloudant/tests/unit/cloudant/test_cloudant_fake.py.
  3. Remove the providers/cloudant/tests/unit/cloudant/test_cloudant_fake.py entry from OVERLOOKED_TESTS in airflow-core/tests/unit/always/test_project_structure.py
  4. These should both pass, the second one confirming that nothing depended on the stub for typing:
breeze testing providers-tests providers/cloudant/tests/unit/cloudant

breeze run mypy providers/cloudant/src/airflow/providers/cloudant

Drafted-by: Claude Code (Opus 5); reviewed and edited by @jroachgolf84 before posting.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions