Skip to content
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

Refactor default_worker_tasks fixture in pytest #215

Closed
wants to merge 2 commits into from

Conversation

imSanko
Copy link
Contributor

@imSanko imSanko commented Feb 26, 2024

Updated the default_worker_tasks fixture in pytest to remove circular dependency and improve readability. The fixture now correctly initializes the set and adds tasks from the tests module.

  • Refactored the default_worker_tasks fixture in pytest to address a circular dependency issue and improve readability. Previously, the fixture had a circular dependency problem where it tried to reference itself within its definition, which could lead to unexpected behavior.

  • The updated code initializes the default_worker_tasks set correctly and then adds tasks from the tests module. This change ensures that the fixture is correctly defined and provides a clear separation of concerns, making the code more maintainable and easier to understand.

imSanko and others added 2 commits February 26, 2024 08:54
Updated the default_worker_tasks fixture in pytest to remove circular dependency and improve readability. The fixture now correctly initializes the set and adds tasks from the tests module.
Copy link

codecov bot commented Feb 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 26.49%. Comparing base (6449d98) to head (57c35ca).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #215   +/-   ##
=======================================
  Coverage   26.49%   26.49%           
=======================================
  Files          36       36           
  Lines        1057     1057           
  Branches      213      213           
=======================================
  Hits          280      280           
  Misses        751      751           
  Partials       26       26           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Nusnus
Copy link
Member

Nusnus commented Feb 26, 2024

Hey there, did you have any actual problems with the original code? Something didn't work as expected?

The tasks set needs to be received as an argument for the pytest fixtures mechanism to work properly, removing it breaks the flow, as the failing tests show.

What did you try to achieve? Maybe I can assist.

P.s
Check out: https://docs.pytest.org/en/latest/reference/fixtures.html#fixtures


@pytest.fixture
def default_worker_tasks(default_worker_tasks: set) -> set:
Copy link
Member

@Nusnus Nusnus Feb 26, 2024

Choose a reason for hiding this comment

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

By receiving the set from "itself", you can populate the set in different fixture availability scopes to control which test has which tasks.

It's a feature not a bug :)

@imSanko imSanko marked this pull request as draft February 26, 2024 06:32
@imSanko imSanko closed this Feb 26, 2024
@imSanko imSanko deleted the patch-2 branch February 26, 2024 06:49
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