Use pytest tmp_dir fixture for safe test paths

[mr-c]

As running the tests post-installation can result in errors otherwise. We need this for the Debian package of toil

Changelog Entry

To be copied to the draft changelog by merger:

• Tests are now more runnable post-installation. Temporary paths are not selected based upon the location of the tests themselves.

Reviewer Checklist

• Make sure it is coming from issues/XXXX-fix-the-thing in the Toil repo, or from an external repo.
  • If it is coming from an external repo, make sure to pull it in for CI with:

    contrib/admin/test-pr otheruser theirbranchname issues/XXXX-fix-the-thing
    

  • If there is no associated issue, create one.
• Read through the code changes. Make sure that it doesn't have:
  • Addition of trailing whitespace.
  • New variable or member names in camelCase that want to be in snake_case.
  • New functions without type hints.
  • New functions or classes without informative docstrings.
  • Changes to semantics not reflected in the relevant docstrings.
  • New or changed command line options for Toil workflows that are not reflected in docs/running/{cliOptions,cwl,wdl}.rst
  • New features without tests.
• Comment on the lines of code where problems exist with a review comment. You can shift-click the line numbers in the diff to select multiple lines.
• Finish the review with an overall description of your opinion.

Merger Checklist

• Make sure the PR passes tests.
• Make sure the PR has been reviewed since its last modification. If not, review it.
• Merge with the Github "Squash and merge" feature.
  • If there are multiple authors' commits, add Co-authored-by to give credit to all contributing authors.
• Copy its recommended changelog entry to the Draft Changelog.
• Append the issue number in parentheses to the changelog entry.

[adamnovak]

This looks OK to me.

[adamnovak]

Good thing we have computers; the CI has informed me that actually this can never work, because it's adding parameters without default values to functions in the tests that start with test_. When pytest goes to run those tests, it doesn't have any values to pass for those arguments, and they fail thusly:

=================================== FAILURES ===================================
_______________ CWLSmallLogDir.test_filename_conflict_resolution _______________
Traceback (most recent call last):
  File "/usr/lib/python3.9/unittest/case.py", line 59, in testPartExecutor
    yield
  File "/usr/lib/python3.9/unittest/case.py", line 592, in run
    self._callTestMethod(testMethod)
  File "/usr/lib/python3.9/unittest/case.py", line 550, in _callTestMethod
    method()
TypeError: test_filename_conflict_resolution() missing 1 required positional argument: 'tmp_path'
---------------------------- Captured stdout setup -----------------------------


[TEST] toil.test.cwl.cwlTest.CWLSmallLogDir:test_filename_conflict_resolution (Dec 06 2022 02:27:46:396579 PST)


------------------------------ Captured log call -------------------------------
INFO     toil.test:__init__.py:113 Setting up toil.test.cwl.cwlTest.CWLSmallLogDir.test_filename_conflict_resolution ...
INFO     toil.test:__init__.py:118 Tore down toil.test.cwl.cwlTest.CWLSmallLogDir.test_filename_conflict_resolution

@mr-c If the path to use isn't going to come from os.path.dirname(__file__), where is it supposed to come from? It doesn't look like you included any calls to these test functions that pass the new argument you are adding.

I think either the new arguments need default values, or the functions need to be renamed to not start with test_, and the old names need to be applied to no-argument functions that compute and supply tmp_dir to the functions that do the actual testing.

[mr-c]

tmp_dir is a built-in pytest fixture https://docs.pytest.org/en/6.2.x/fixture.html

[mr-c]

So maybe someplace in the CI we are running the tests using unittest instead of pytest, or there is some other issue with how the Toil tests are structured.

[mr-c]

@adamnovak I converted the tmp_path using tests to an idiomatic pytest style; should work now

[adamnovak]

This can probably work? I'm worried that if we do everything like this we'll end up with a lot of marks, though, and we won't get to use the stuff we put in ToilTest.

[adamnovak]

Does this mark mean anything about the tests (like they need something) or is it just a category of tests so we can run them?

[mr-c]

I don't know, I didn't write the original test classes (CWLSmallLogDir , CWLSmallTests). I added the marks so that the cwl_misc gitlab job could still select them. We would split the test categories by files instead, then marks wouldn't needed.

[mr-c]

@adamnovak If you want, I can turn the ToilTest class attributes/methods into pytest fixtures in a separate PR. That will be more flexible and explicit.