tests: stop using tmp_dir fixture in favor of pytest.tmp_path fixture

[martin-kokos]

Pull Request Check List

Resolves: #6934

• Added tests for changed code.
• Updated documentation for changed code.

[martin-kokos]

rebased to master

[eamanu]

There's an old PR #7129 but From last year and it was not updated.

[eamanu]

As @radoering noted the current CI tests in windows has a similar fail with the other PR #7129.

[martin-kokos]

I have added some code for debugging the failed Windows pipelines because the error output is truncated.
The problem appears to be the pytest.tmp_path path prefix making the file paths too long.

Cloning into 'C:/Users/runneradmin/AppData/Local/Temp/pytest-of-runneradmin/pytest-0/popen-gw0/test_system_git_called_when_co0/.cache/pypoetry/src/test-fixture-vcs-repository/submodules/sample-namespace-packages'...
fatal: cannot write keep file 'C:/Users/runneradmin/AppData/Local/Temp/pytest-of-runneradmin/pytest-0/popen-gw0/test_system_git_called_when_co0/.cache/pypoetry/src/test-fixture-vcs-repository/.git/modules/submodules/sample-namespace-packages/objects/pack/pack-c638ec21e7fcb21acf4e0b89cefcdb31ab0adc29.keep': Filename too long

https://github.com/python-poetry/poetry/actions/runs/4364500429/jobs/7631897284#step:14:2832

[eamanu]

Take a look here pytest-dev/pytest#8999

[radoering]

Just some minor nitpicks for consistency. I think after that we are ready to merge. 😄

[dimbleby]

#7784 kicks in and tells us something about the flakey CI!

https://github.com/python-poetry/poetry/actions/runs/4669647501/jobs/8268387674?pr=7412

Traceback (most recent call last):
  File "C:\Users\RUNNER~1\AppData\Local\Temp\tmpmn5fwvjz\.venv\lib\site-packages\build\__init__.py", line 466, in _handle_backend
    yield
  File "C:\Users\RUNNER~1\AppData\Local\Temp\tmpmn5fwvjz\.venv\lib\site-packages\build\__init__.py", line 356, in get_requires_for_build
    return set(get_requires(config_settings))
  File "C:\Users\RUNNER~1\AppData\Local\Temp\tmpmn5fwvjz\.venv\lib\site-packages\pyproject_hooks\_impl.py", line 166, in get_requires_for_build_wheel
    return self._call_hook('get_requires_for_build_wheel', {
  File "C:\Users\RUNNER~1\AppData\Local\Temp\tmpmn5fwvjz\.venv\lib\site-packages\pyproject_hooks\_impl.py", line 311, in _call_hook
    self._subprocess_runner(
  File "C:\Users\RUNNER~1\AppData\Local\Temp\tmpmn5fwvjz\.venv\lib\site-packages\build\__init__.py", line 302, in _runner
    self._hook_runner(cmd, cwd, extra_environ)
  File "C:\Users\RUNNER~1\AppData\Local\Temp\tmpmn5fwvjz\.venv\lib\site-packages\pyproject_hooks\_impl.py", line 71, in quiet_subprocess_runner
    check_output(cmd, cwd=cwd, env=env, stderr=STDOUT)
  File "C:\hostedtoolcache\windows\Python\3.9.13\x64\lib\subprocess.py", line 424, in check_output
    return run(*popenargs, stdout=PIPE, timeout=timeout, check=True,
  File "C:\hostedtoolcache\windows\Python\3.9.13\x64\lib\subprocess.py", line 528, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['C:\\Users\\runneradmin\\AppData\\Local\\Temp\\build-env-k65ld4h9\\Scripts\\python.exe', 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmpmn5fwvjz\\.venv\\lib\\site-packages\\pyproject_hooks\\_in_process\\_in_process.py', 'get_requires_for_build_wheel', 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmp95wnwt0t']' returned non-zero exit status 1.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 16, in <module>
  File "C:\Users\RUNNER~1\AppData\Local\Temp\tmpmn5fwvjz\.venv\lib\site-packages\build\__init__.py", line 356, in get_requires_for_build
    return set(get_requires(config_settings))
  File "C:\hostedtoolcache\windows\Python\3.9.13\x64\lib\contextlib.py", line 137, in __exit__
    self.gen.throw(typ, value, traceback)
  File "C:\Users\RUNNER~1\AppData\Local\Temp\tmpmn5fwvjz\.venv\lib\site-packages\build\__init__.py", line 474, in _handle_backend
    raise BuildBackendException(  # noqa: B904 # use raise from
build.BuildBackendException: Backend subprocess exited when trying to invoke get_requires_for_build_wheel

Not sure what to make of that to be honest, but it's some sort of progress

Windows 3.9 is the one that's stuck on old virtualenv, possibly relevant

[radoering]

test_info_setup_missing_mandatory_should_trigger_pep517[name]

It's the test case with the missing name in setup.py.

error: Invalid distribution name or version syntax: poetry-config-5k08imt--0.1.0

Is it possible that the name of some temporary directory is used as package name and that if we are unlucky this name ends with a dash so that we get an invalid name? (Can't look for myself right now.)

[martin-kokos]

#7784 kicks in and tells us something about the flakey CI!

I have rebased to pull it in.

[dimbleby]

I have rebased to pull it in.

As previously discussed - #7412 (comment) - the pipeline was doing that already

[martin-kokos]

Rebased due to conflict

[dimbleby]

flakey test is on windows 3.10 this time, and mentions poetry-config-fxu7gvb--0.1.0 - so that knocks out the virtualenv theory and is consistent with the "awkwardly named temporary directory" theory.

Given that this MR seems to be hitting this problem much more than any other branch, and the hint that it's to do with the name of a temporary directory... probably you need to get to the bottom of this one - or at least far enough to show that it's not related to this MR - before merging.

[radoering]

It seems that the failure is really caused by this PR:

After setting a breakpoint at

poetry/tests/inspection/test_info.py

Line 236 in 5a9da19

setup = "from setuptools import setup; "

I noticed that source_dir is empty without this PR, but contains two directories .cache and something like poetry_config_jz_t48_l with this PR.

If there is no name in setup.py and there is exactly one directory (directories starting with a dot don't count as it seems) next to setup.py, setuptools assumes that's the name. If there are two folders setuptools raises an exception. If there is no folder, setuptools sets the name to UNKNOWN.

Thus, the question is what change of this PR creates these two directories?

[radoering]

OK, it's clear now: config_cache_dir and config_dir in conftest.py used tmp_dir before and source_dir in test_info.py used tmp_path, so we had two different directories and now it's all the same.

I suppose we should change

poetry/tests/inspection/test_info.py

Lines 38 to 39 in 5a9da19

	def source_dir(tmp_path: Path) -> Path:
	return Path(tmp_path.as_posix())

-    return Path(tmp_path.as_posix())
+    path = tmp_path / "source"
+    path.mkdir()
+    return path

[martin-kokos]

OK, it's clear now: config_cache_dir and config_dir in conftest.py used tmp_dir before and source_dir in test_info.py used tmp_path, so we had two different directories and now it's all the same.

I suppose we should change

poetry/tests/inspection/test_info.py

Lines 38 to 39 in 5a9da19

	def source_dir(tmp_path: Path) -> Path:
	return Path(tmp_path.as_posix())

-    return Path(tmp_path.as_posix())
+    path = tmp_path / "source"
+    path.mkdir()
+    return path

Thanks a lot for the investigation. I don't think I would've been able to figure it out on my own, especially on Win. Is there a way to trigger CI re-run on GH, to make sure its not passing by chance since it was manifesting itself randomly?

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.