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

Mild executor fixture refactor #1705

Merged
merged 1 commit into from
Nov 4, 2024
Merged

Mild executor fixture refactor #1705

merged 1 commit into from
Nov 4, 2024

Conversation

khk-globus
Copy link
Contributor

In 4f6e7aa (#1421), the gc_executor started using MockedExecutor. This also removed the need to create a separate mocked Client. To keep the change at the time focused on that, the changeset did not remove the now-superfluous gcc part of the yield.

This changeset now cleans up that refactor, and the core of this change is:

-def gc_executor(mock_result_watcher):
-    gce = MockedExecutor()
-    yield gce.client, gce
+def gce(mock_result_watcher):
+    gc_executor = MockedExecutor()
+    yield gc_executor

The yield statement is the key change, but the renaming of the fixture is intended to reduce the other necessary changes, since the tests that use this fixture use gce as their executor variable.

Type of change

  • Code maintenance/cleanup

In 4f6e7aa, the `gc_executor` started using
`MockedExecutor`.  This also removed the need to create a separate mocked
Client.  To keep the change at the time focused on that, the changeset did not
remove the now-superfluous `gcc` part of the yield.

This changeset now cleans up that refactor, and the core of this change is:

```diff
-def gc_executor(mock_result_watcher):
-    gce = MockedExecutor()
-    yield gce.client, gce
+def gce(mock_result_watcher):
+    gc_executor = MockedExecutor()
+    yield gc_executor
```

The `yield` statement is the key change, but the renaming of the fixture is
intended to reduce the other necessary changes, since the tests that use this
fixture use `gce` as their executor variable.
@khk-globus khk-globus added no-news-is-good-news This change does not require a news file quick-review Review of this should be quick and easy labels Nov 4, 2024
@khk-globus khk-globus merged commit 86a1002 into main Nov 4, 2024
17 checks passed
@khk-globus khk-globus deleted the nochange_test_refactor branch November 4, 2024 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-news-is-good-news This change does not require a news file quick-review Review of this should be quick and easy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants