Skip to content

Enable ruff B015 to catch silent no-op comparisons in tests#66977

Merged
shahar1 merged 5 commits into
apache:mainfrom
shahar1:enable-ruff-b015-useless-comparison
May 16, 2026
Merged

Enable ruff B015 to catch silent no-op comparisons in tests#66977
shahar1 merged 5 commits into
apache:mainfrom
shahar1:enable-ruff-b015-useless-comparison

Conversation

@shahar1
Copy link
Copy Markdown
Contributor

@shahar1 shahar1 commented May 15, 2026

Bare actual == expected expressions in tests silently evaluate and discard their result — the test passes regardless of correctness. This is exactly what happened in #66894 (four test_serialize methods in the Vertex AI triggers had been vacuously passing for years).

Changes

  • pyproject.toml: add B015 (useless-comparison) to extend-select. This rule fires on any bare comparison expression used as a statement.
  • 16 test files: add the missing assert to all 26 existing violations found by the rule across airflow-core, task-sdk, and providers amazon, cncf/kubernetes, google).

With this in place, any future bare comparison in a test is a lint error caught before CI runs.


Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Sonnet 4.6)

Generated-by: Claude Code (Sonnet 4.6) following the guidelines

@boring-cyborg boring-cyborg Bot added area:CLI area:DAG-processing area:providers area:Scheduler including HA (high availability) scheduler area:task-sdk provider:amazon AWS/Amazon - related issues provider:cncf-kubernetes Kubernetes (k8s) provider related issues provider:google Google (including GCP) related issues labels May 15, 2026
@shahar1 shahar1 requested a review from vatsrahul1001 May 15, 2026 04:59
Bare `actual == expected` expressions (missing `assert`) silently pass
regardless of correctness. Enable ruff rule B015 (useless-comparison)
to make this a lint error, and add `assert` to all 26 existing instances
found across airflow-core, task-sdk, and providers (amazon, cncf, google).
@shahar1 shahar1 force-pushed the enable-ruff-b015-useless-comparison branch from ff2e36f to 7dfec04 Compare May 15, 2026 05:01
@shahar1 shahar1 requested review from Lee-W and jason810496 May 15, 2026 05:20
@shahar1 shahar1 marked this pull request as draft May 15, 2026 05:36
`.calls` is not a real MagicMock attribute; accessing it auto-creates a
child MagicMock. The expression `mock.calls[0] == []` was a silent no-op
that B015 correctly caught. Adding `assert` exposed that the comparison
always evaluates to False.

Replace with `assert_not_called()`, which correctly verifies that
`revoke_task` routes through `kube_scheduler.patch_pod_revoked()` rather
than calling `patch_namespaced_pod` directly.
@shahar1 shahar1 marked this pull request as ready for review May 15, 2026 05:50
@shahar1 shahar1 marked this pull request as draft May 15, 2026 06:29
- test_bedrock: set ensure_unique_job_name on the operator before execute
  so the not-ensure-unique parametrize cases actually test the right path,
  and use pytest.raises for the cases where a conflict error is expected
- test_emr_serverless (create): remove dead get_application mock setup and
  wrong call-count assertion; the operator uses waiters, not direct polling
- test_emr_serverless (start job): correct expected call_count from 2 to 1;
  the operator calls get_application once (state CREATED is in SUCCESS_STATES
  so no retry loop)
@shahar1 shahar1 marked this pull request as ready for review May 15, 2026 06:37
Copy link
Copy Markdown
Contributor

@jscheffl jscheffl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool! I assume you can work on making CI green in parallel :-D

- Add missing `assert` to three bare comparisons caught by the new
  B015 rule: test_pod_template_file.py, test_otel_logger.py (×2)
- Fix the MappedOperator task_type assertion in test_dag_serialization:
  `type(task).__name__` gives the wrapper class (`DecoratedMappedOperator`)
  but `operator_class["task_type"]` stores the underlying operator's name
  (`task.operator_class.__name__`, e.g. `_PythonDecoratedOperator`)
@shahar1 shahar1 requested a review from potiuk as a code owner May 15, 2026 15:55
@shahar1 shahar1 requested a review from bugraoz93 as a code owner May 15, 2026 15:55
The operator uses json.dump() which writes strings incrementally to the
file handle; the previous assertion expected bytes in a single write call.
Also adds the missing mock_file.write.reset_mock() between the CSV and
JSON sub-tests so write calls don't accumulate across sections.
@shahar1 shahar1 merged commit 2ed6805 into apache:main May 16, 2026
292 checks passed
@shahar1 shahar1 deleted the enable-ruff-b015-useless-comparison branch May 16, 2026 05:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:CLI area:DAG-processing area:providers area:Scheduler including HA (high availability) scheduler area:task-sdk provider:amazon AWS/Amazon - related issues provider:cncf-kubernetes Kubernetes (k8s) provider related issues provider:google Google (including GCP) related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants