Replaced --forked in internal tests

[utsavm9]

Resolves #107:

• pytest-xdist will now be installed on Windows
• Replaced --forked with -n 2
• Tests like test_basic_runWithReturn_pass and 4 others do not want to have the file __cell_coverage_test.py already created. I grouped all these 5 tests under a class TestCoverage so that these tests are run sequentially by a worker spawn by pytest-xdist
• Removed special handling of --forked. Now, tests like test_state behave as if it's always FORKED=False.

Naming Choices:

• Would you want to rename make pytest-forked to something else?
• Azure test pytest --forked renames to pytest -n 2
• Is the class name TestCoverage apt for grouping these tests? test_basic_runWithReturn_pass, test_basic_runWithReturn_fail, test_basic_runWithReport_pass, test_basic_runWithHTMLReturn_pass, test_basic_runWithHTMLReturn_fail

It is not apparent looking at the diff of test_test.py TestCoverage, but I have only added those five tests to the class and I have not changed those tests.

[timkpaine]

these are failing lint, run make fix for auto fix

[utsavm9]

Please can you explain again why you are grouping these?

You guessed it right, without grouping the tests will not always pass. Those tests first check if __cell_coverage_test.py has not already been created and fail if it already exists. The file would be present if another of those 5 tests is running in another worker. https://github.com/jpmorganchase/nbcelltests/blob/aef8e3274edd8ec6ed84e0866b555c6e72c4a50b/nbcelltests/tests/test_test.py#L690

---

So, those 5 tests cannot be parallelized. One way to make pytest-xdist run those sequentially is to group them in a class and pass --dist=loadscope. From the docs:

--dist=loadscope: Tests are grouped by module for test functions and by class for test methods. Groups are distributed to available workers as whole units.

[ceball]

Quick comment from my phone...

I think those tests (or the underlying functions they test) should be fixed. E.g. they should use a unique temporary file.

We should either do that here in this PR, or open an issue to do it in the future. Up to you! (Off the top of my head I don't know if it's a big change or not.)

[utsavm9]

I am not sure at all why TestSkips is failing to setup once on Linux Python37. I am trying to duplicate that on my local machine.

Let's open another issue to make those tests independent. There are two files involved: __cell_coverage_test.py and _cell_coverage_test.py, so it will take a while and I can get back to it after some time.

[ceball]

I am not sure at all why TestSkips is failing to setup once on Linux Python37. I am trying to duplicate that on my local machine.

Looked like a timeout to me, probably just from azure. So I re-ran, and it passed.

(If it was down to some bad interaction between the tests that only shows up sometimes in the distributed case, we'll address it if we see it again in the future...)

[utsavm9]

One last thing to ask: Would you like to be able to do make testpy-distributed N=8 along with make testpy-distributed? I personally have been using N=4 on my university's Linux server to test my changes as that was the fastest on that machine. See this: https://github.com/utsavm9/nbcelltests/commit/c934476b3cc8e89bc210c33c6bb7d7a0bab86bb5

Apart from that, I am all done with this PR!

[ceball]

Let's open another issue to make those tests independent. There are two files involved: __cell_coverage_test.py and _cell_coverage_test.py, so it will take a while and I can get back to it after some time.

Are you able to make the issue (extracting and summarizing our discussion here)? (No obligation; if not, I will do it.)

[ceball]

One last thing to ask: Would you like to be able to do make testpy-distributed N=8

Please could you make that as a separate PR, after this one? I am not at all against adding an option for varying n when running the tests (I think it's useful), but I don't own the Makefile so would want a teammate to ok it.

[utsavm9]

OK, I will make another PR to master after this PR is merged. I can make the issue about those tests too. Thanks!

[ceball]

I'll merge once the tests pass (can't merge until then).