Use tmp_path instead of tmpdir pytest fixture

[jdavies-st]

Replace all instances tmpdir and tmpdir_factory pytest fixtures with tmp_path and tmp_path_factory fixtures, respectively.

Then in pyproject.toml:

[tool.pytest.ini_options]
addopts = [
    "-p no:legacypath",
]

which will prevent it from being used in the future. This is will make the code more robust for use of pathlib.Path instances in the runtime code.

[nden]

@stscijgbot-jp

[stscijgbot-jp]

This issue is tracked on JIRA as JP-3552.

[stscijgbot-jp]

Comment by Ned Molter on JIRA:

for doctests, following example from Astropy: pytest discussion question from pllim

[stscijgbot-jp]

Comment by Ned Molter on JIRA:

The use of "-p no:legacypath" currently causes failures when running tests with both the GitHub Action and Jenkins that are due to the dependency on ci_watson.  ci_watson was updated 4 days ago to remove instances of "tmpdir" and use "-p no:legacypath" but a release has not yet been published.  Brett Graham mentioned that ci_watson is little maintained and could possibly be replaced in the future.  He suggested two possible paths forward:

• apply similar fixes to jwst as was in this romancal PR (this would duplicate fixtures in these two packages but allow each to make their own modifications)
• update ci_watson to make _jail public and use it in both packages

Does anyone have opinions on which of these is the preferred option?

[stscijgbot-jp]

Comment by Tyler Pauly on JIRA:

I don't know much about ci_watson, but its description sounds as though its for Jenkins testing - is it also used for GitHub Actions? Our future testing plans include a migration from Jenkins to GH Actions - will we still be using ci_watson when that happens? If so, it may make sense to get an update into it - if not, probably best not to worry about that side and get the non-ci_watson fix included. Not sure if the proposed replacement package that James brought up will be used or not.

All this to say - it may make sense to ask Joe Hunkeler or Zach Burnett, as the choice is probably weighted most heavily by the future of our test architecture.

[stscijgbot-jp]

Comment by Joe Hunkeler on JIRA:

Repositories that deal with input data and "truth" files for testing use ci_watson to generate Artifactory "spec" files. When a test failure occurs pytest's basetemp contains the (potential truth) files. These paths are recorded in a JSON config. Jenkins uses an Artifactory plugin to process the configs, but honestly, it's equivalent to running this in an Action:

find $PATH_TO_PYTEST_BASETEMP -type -name 'result_*.json' | xargs -I '{}' jf rt u --spec '{}'

[jdavies-st]

I would drop the use of ci_watson to provide _jail and just write your own tmp_cwd based on pytest tmp_path fixture. Just use ci_watson for the time being for its Artifactory interface tools. They are really separate things and don't depend on one another.

Making a tmp_cwd pytest fixture to replace _jail is easy. I've done it for astroquery, and another package I maintain:

https://github.com/mpi-astronomy/snowblind/blob/b58f5b30287737c84ce0b5624af81c6f65f4a1c1/tests/conftest.py#L7-L15

[stscijgbot-jp]

Comment by Ned Molter on JIRA:

Ok, thanks both of you!  The tmp_cwd fixture is basically the same as the function_jail that Roman implemented, but I'm going to use tmp_cwd because I think the name is more descriptive.  And thanks for the clarification about how those two uses of ci_watson are independent, I agree it is beyond the scope of this ticket (if even desired at present) to replace the artifactory interface tools

[jdavies-st]

There's also a module-scoped pytest fixture called jail in this package

jwst/jwst/conftest.py

Lines 54 to 69 in 911b5c6

	@pytest.fixture(scope="module")
	def jail(request, tmpdir_factory):
	"""Run test in a pristine temporary working directory, scoped to module.
	This fixture is the same as _jail in ci_watson, but scoped to module
	instead of function. This allows a fixture using it to produce files in a
	temporary directory, and then have the tests access them.
	"""
	old_dir = os.getcwd()
	path = request.module.__name__.split('.')[-1]
	if request._parent_request.fixturename is not None:
	path = path + "_" + request._parent_request.fixturename
	newpath = tmpdir_factory.mktemp(path)
	os.chdir(str(newpath))
	yield newpath
	os.chdir(old_dir)

which is used extensively in the regression tests, and it would be worthwhile changing that one to use tmp_path_factory as well. And if you're improving the names to something descriptive, I'd recommend changing its name to something descriptive like tmp_cwd_module_scoped or something more catchy if like.

[stscijgbot-jp]

Comment by Ned Molter on JIRA:

Thanks, yep, I already updated that one

[stscijgbot-jp]

Comment by Howard Bushouse on JIRA:

Fixed by #8327