Resolve order dependency between test cases

[kard3n]

Some of the test cases in tests/test_coroutine_sink.py set the spawn methods for the multiprocessing module, but do not reset it afterwards. This causes some tests in the tests/test_add_option_context.py class to fail, as they assume the spawn method to be "None" beforehand.

For example, the following will fail under Linux:
pytest tests/test_coroutine_sink.py::test_enqueue_coroutine_from_inside_coroutine_without_loop tests/test_add_option_context.py::test_fork_context_as_object[fork].

To address this, I have added a fixture to the polluting test cases in tests/test_coroutine_sink.py that resets the start method after they've been executed. Additionally, I've modified a pre-existing fixture in tests/test_add_option_context.py that resets the start method both before and after the tests that check for it (or potentially modify it) are executed.

[Delgan]

Good catch, thanks!

Actually, the exact same fix was part of a branch I've not yet merged: master...refactor-enqueue-new#diff-d606c3cab8620dda9dbef0f83d25d11f5d8a19cfa1d39fc9be7ac52d4e8cae5f

What do you think of also adding autouse=True to the reset_start_method() fixture? This would prevent future regressions if we ever forget to use the fixture manually.

[kard3n]

That's a great idea! I've set the reset_start_method() fixture to autouse=True in latest commit.

[Delgan]

Thanks for the update. 👍

One last thing: can you please clarify to me if there is any intended difference between reset_multiprocessing_start_method() and reset_start_method()?

[kard3n]

Somewhat. I originally thought that test_coroutine_sink.py was the only class setting the multiprocessing start method, and decided that it would be a good idea if it cleaned up after itself. So reset_multiprocessing_start_method() was thought to be only a post-execution cleanup fixture for the polluting methods, so that if any other test does checks against the start method it would not fail.

After your comment I checked again, and it turns out that more test classes have test cases that set the start method, so I think it would be a good idea to either:

• Remove the reset_multiprocessing_start_method() fixture, since the affected tests in test_add_option_context.py clear it themselves
• Or add it to all other test cases that set the start method for extra security

What are your thoughts?

[Delgan]

Yeah, I think we can go ahead and apply the patch globally by defining the fixture with autouse=True in conftest.py. This will ensure that the tests remain deterministic in the future, regardless of the order of execution.

I would consider this a safeguard workaround for a missing feature in multiprocessing. Specifically, it is unfortunately not possible to use a default context without also setting it globally as a side effect (python/cpython#109070).

[kard3n]

Done. I've added a new reset_multiprocessing_start_method() fixture to conftest.py. It contains the code of the old reset_start_method() fixture, but "reset_multiprocessing_start_method" explains what it does a bit clearer.

And hopefully the missing feature you mentioned will be fixed/added soon, but since this library maintains backwards compatibility it sadly won't change much for the time being.

[Delgan]

That's great, thanks again for your help!