Skip to content

Remove duplicate 'mock_supervisor_comms' pytest fixture definitions#49457

Merged
amoghrajesh merged 11 commits intoapache:mainfrom
astronomer:cleanup_mock_supervisor_comms
Apr 22, 2025
Merged

Remove duplicate 'mock_supervisor_comms' pytest fixture definitions#49457
amoghrajesh merged 11 commits intoapache:mainfrom
astronomer:cleanup_mock_supervisor_comms

Conversation

@prabhusneha
Copy link
Copy Markdown
Contributor

@prabhusneha prabhusneha commented Apr 19, 2025

Copy link
Copy Markdown
Contributor

@amoghrajesh amoghrajesh left a comment

Choose a reason for hiding this comment

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

We have a couple more occurrences of this, in:

  • airflow-core/tests/conftest.py
  • providers/openlineage/tests/unit/openlineage/plugins/test_listener.py (create a seperate PR for this please)

@prabhusneha
Copy link
Copy Markdown
Contributor Author

@amoghrajesh These are the locations where mock_supervisor_comms is defined currently in main.

  1. airflow-core/tests/conftest.py
  2. airflow-core/tests/unit/hooks/test_base.py
  3. airflow-core/tests/unit/models/test_taskinstance.py
  4. devel-common/src/tests_common/pytest_plugin.py
  5. providers/openlineage/tests/unit/openlineage/plugins/test_listener.py

I had initially retained it in 1, so it can be removed from 2 and 3.
Where are you suggesting it be moved for 4 and 5 as well?

@amoghrajesh
Copy link
Copy Markdown
Contributor

@amoghrajesh These are the locations where mock_supervisor_comms is defined currently in main.

  1. airflow-core/tests/conftest.py
  2. airflow-core/tests/unit/hooks/test_base.py
  3. airflow-core/tests/unit/models/test_taskinstance.py
  4. devel-common/src/tests_common/pytest_plugin.py
  5. providers/openlineage/tests/unit/openlineage/plugins/test_listener.py

I had initially retained it in 1, so it can be removed from 2 and 3. Where are you suggesting it be moved for 4 and 5 as well?

If we have it in the pytest_plugin.py, it means it can be accessed from anywhere in the codebase. So you should be able to remove every other occurence. For the providers, make a different PR, rest can be in this one

Copy link
Copy Markdown
Contributor

@amoghrajesh amoghrajesh left a comment

Choose a reason for hiding this comment

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

LGTM +1

@amoghrajesh amoghrajesh merged commit 493402d into apache:main Apr 22, 2025
50 checks passed
@amoghrajesh amoghrajesh deleted the cleanup_mock_supervisor_comms branch April 22, 2025 07:32
prabhusneha pushed a commit to astronomer/airflow that referenced this pull request Apr 25, 2025
github-actions bot pushed a commit that referenced this pull request Jun 23, 2025
…from core (#49457)

(cherry picked from commit 493402d)

Co-authored-by: Sneha Prabhu <sneha.prabhu@astronomer.io>
@github-actions
Copy link
Copy Markdown

Backport successfully created: v3-0-test

Status Branch Result
v3-0-test PR Link

ashb pushed a commit that referenced this pull request Jun 23, 2025
…from core (#49457) (#52080)

(cherry picked from commit 493402d)

Co-authored-by: Sneha Prabhu <sneha.prabhu@astronomer.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants