-
Notifications
You must be signed in to change notification settings - Fork 16.3k
Remove global from listener unit tests #58838
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove global from listener unit tests #58838
Conversation
8a661f2 to
89e0f79
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR removes global statements from listener unit test modules, replacing them with a cleaner pattern using functools.cache() and dataclasses. The changes improve code maintainability by eliminating global state in favor of a cached singleton pattern.
- Introduced
ListenerStatedataclass to encapsulate listener state - Replaced global variables with a cached
get_listener_state()function - Updated test files to access listener state through the new getter function
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| airflow-core/tests/unit/listeners/full_listener.py | Refactored from global variables to dataclass-based state with @cache decorator for singleton pattern |
| airflow-core/tests/unit/listeners/lifecycle_listener.py | Refactored from global variables to dataclass-based state with @cache decorator for singleton pattern |
| airflow-core/tests/unit/listeners/test_listeners.py | Updated assertions to access listener state via get_listener_state() method |
| airflow-core/tests/unit/jobs/test_base_job.py | Updated assertions to access listener state via get_listener_state() method |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* Remove global from listener unit tests * Fix dataclass * Copilot review feedback Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Copilot review feedback Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* Remove global from listener unit tests * Fix dataclass * Copilot review feedback Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Copilot review feedback Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* Remove global from listener unit tests * Fix dataclass * Copilot review feedback Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Copilot review feedback Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Another small increment to remove global statements for PR #58116
This just removes some global statements in unit tests, replaces with
functools.cache()globalis evil.