jroachgolf84 opened a new issue, #72276: URL: https://github.com/apache/airflow/issues/72276
Two modules in the `google` provider have no dedicated test module. Nothing under any `providers/*/tests/` directory imports either of them: | Module | Expected test file | | --- | --- | | `providers/google/src/airflow/providers/google/cloud/utils/bigquery.py` | `providers/google/tests/unit/google/cloud/utils/test_bigquery.py` | | `providers/google/src/airflow/providers/google/cloud/utils/bigquery_get_data.py` | `providers/google/tests/unit/google/cloud/utils/test_bigquery_get_data.py` | Both are currently suppressed in the `OVERLOOKED_TESTS` allowlist in [`airflow-core/tests/unit/always/test_project_structure.py`](https://github.com/apache/airflow/blob/main/airflow-core/tests/unit/always/test_project_structure.py). This is a scoped subset of the meta issue #35442, limited to the BigQuery helpers under `google/cloud/utils`. ### What should these tests cover? Worth being precise about the current state, because it differs between the two modules and neither is quite "completely untested". `bq_cast` and `convert_job_id` do get executed incidentally, through `BigQueryCursor` tests such as `test_next` in `providers/google/tests/unit/google/cloud/hooks/test_bigquery.py` and through `BigQueryInsertJobOperator` tests. But no test asserts their own semantics, and the interesting cases are all unexercised. `bigquery_get_data` is worse than untested. The two places that call it, `bigquery_to_postgres.py` and `bigquery_to_sql.py`, are tested with the function patched out (`mock.patch("airflow.providers.google.cloud.transfers.bigquery_to_postgres.bigquery_get_data")`), so the real body never runs in CI at all. For `bigquery.py`: - `bq_cast` returns `None` for a `None` input regardless of the declared type - `INTEGER` casts to `int`, and `FLOAT` casts to `float` - `TIMESTAMP` also casts to `float`, which is deliberate but surprising enough that it should be pinned rather than left implicit - `BOOLEAN` maps the strings `true` and `false` to real booleans, and anything else raises `ValueError` with the expected message - an unrecognised type falls through and returns the string unchanged - `convert_job_id` builds `project_id:location:job_id` for a single id, maps over the list form and returns a list, and defaults `location` to `US` when it is `None` For `bigquery_get_data.py`: - successive batches are yielded as lists of row values, with `start_index` advancing by `batch_size` on each call to `BigQueryHook.list_rows` - the generator terminates when a batch comes back empty, which is the only exit from the `itertools.count` loop, so a test that never returns an empty batch would hang - `selected_fields` and `batch_size` are passed through to `list_rows` unmodified - a `RowIterator` return raises `TypeError`, which is the guard against someone calling the hook with `return_iterator=True` Two conventions from the contributing docs apply here. The hook should be mocked with `autospec` rather than a bare `Mock`, and since this module logs through an injected logger, any assertion on that should use structured `caplog` checks rather than matching raw log text. ## Definition of Done 1. Add the two test modules at the paths in the table above. 2. Remove the two corresponding entries from `OVERLOOKED_TESTS` in `airflow-core/tests/unit/always/test_project_structure.py` 3. Both of these tests should pass: ```bash breeze testing providers-tests providers/google/tests/unit/google/cloud/utils/test_bigquery.py breeze testing providers-tests providers/google/tests/unit/google/cloud/utils/test_bigquery_get_data.py ``` --- Drafted-by: Claude Code (Opus 5); reviewed and edited by @jroachgolf84 before posting -- 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]
