[bug] not playing well with discovering unittests

[liamhuber]

Currently all the unit tests use the paradigm cd tests; python -m unittest discover ., however a perfectly reasonable alternative is python -m unittest discover tests. However, the latter fails with error messages like

Traceback (most recent call last):
  File "/home/runner/work/pympipool/pympipool/pympipool/backend/mpiexec.py", line 90, in <module>
    main()
  File "/home/runner/work/pympipool/pympipool/pympipool/backend/mpiexec.py", line 46, in main
    input_dict = interface_receive(socket=socket)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/share/miniconda3/envs/test/lib/python3.11/site-packages/pympipool/shared/communication.py", line 147, in interface_receive
    return cloudpickle.loads(socket.recv())
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ModuleNotFoundError: No module named 'test_meta'
Error: The action has timed out.

As can be seen in #238 Unittests_by_path.

To the best of my understanding, this is a result of these lines in the backend:

https://github.com/pyiron/pympipool/blob/2c616aced49e8d513f3fd18aefda6f2d0d41f4e8/pympipool/backend/serial.py#L22-L26

Since in the latter case "." is the parent path from which discover is invoked, and not the path where the actual test files reside.

This can be fixed by adding the test directory(ies) to the path (also demonstrated in #238 Unittests_by_path_with_env and Unittests_by_path_with_pythonpath which both run fine), but it still seems to me like something that should be fixed at the source rather than patched over in github workflows. In particular, I worry that if unittest discover has this sort of problem, other ways users invoke the executors might be similarly bugged.

It's not clear to me whether or not there is a good solution to this problem.

Aside: the comment in the snippet above says this is necessary for flux, but if this is really the relevant line for this issue that would imply it is important for all the executors, or?

[liamhuber]

@jan-janssen do you have any insight here? The solutions I see, in decreasing order of desirability, are:

• Change the centralized CI to accommodate pympipool (eg by always adding a bunch of directories to the python path)
• Abandoning the centralized CI altogether for any repo using pympipool executors
• Not use pympipool executors

But frankly even the best of those is not particularly nice.

It's not clear to me how the insertion of "." to the path could be made universally robust, but this would be the "right" solution if it's possible.

[jan-janssen]

@liamhuber I am in the same position, I am not aware of a way to transfer the classes. We could try to always pickle by value rather than by reference, but that is not as computationally efficient, so I do not want this as the default. Also changing the tests sounds annoying to me. Maybe somebody can take a look what kind of references cloudpickle receives when unittest is executing a test, we could try to recognise classes from unittest and then always pickle by value.

[liamhuber]

@jan-janssen, ok thanks. It's nice that I've understood the situation at least 😂

I agree simply pickling everything by value is not the way to go and for the reasons you give.

Filtering by unittest classes is an interesting idea. I worry it treats the symptom and not the cause, i.e. that there are other conditions where the code gets invoked from not-"." leading to the same problem. However, on this front the good news is I couldn't manage to cook up a situation where that happened; I tried a setup with some_dir, and some_subdir where the subdir had python leveraging the executors, but it always worked fine regardless of the misdirection of invoking it from the parent dir with a bash script, or invoking it from a subdir bash script which is in turn invoked from a parent dir bash script. Of course that's not proof that we will never run into the problem anywhere else, but at least I couldn't trigger it trivially.

Github reusable workflows have explicit limitations around passing env variables from calling workflows to reusable workflows, but I'm confident we can find a workaround -- at least a boolean flag that triggers adding the stuff from inside the called workflow. IMO this is the least-worst solution, so I'll move in that direction.