Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a single tmpdir fixture for all supported Python versions #2027

Merged
merged 1 commit into from
Jun 29, 2023

Conversation

happz
Copy link
Collaborator

@happz happz commented Apr 26, 2023

See comments in conftest.py, but in general:

  • because of our code, we would like to use tmp_path and Path it returns,
  • but we have also our custom tmt.utils.Path we should be using instead of the original pathlib.Path,
  • because of RHEL8, we also get pytest without tmp_path fixture,
  • and tmpdir fixture returns py.path.local.

The patch adds our custom fixture which returns tmt.utils.Path no matter what pytest we get. It's achieved by a test import, trivial wrapper class, and two fixtures hiding the actual pytest fixture involved.

@happz happz added this to the 1.24 milestone Apr 26, 2023
@happz happz force-pushed the unit-test-tmp-path-wrappers branch 5 times, most recently from 9de95ad to ba8cb69 Compare May 15, 2023 14:39
@happz happz added the no functional change "No Functional Change" intended. Patch should not change tmt's behavior in any way. label May 22, 2023
@happz happz force-pushed the unit-test-tmp-path-wrappers branch from ba8cb69 to b59db88 Compare May 26, 2023 13:28
@happz happz modified the milestones: 1.24, 1.25 Jun 5, 2023
@happz happz force-pushed the unit-test-tmp-path-wrappers branch 2 times, most recently from cca11d9 to 8075d7e Compare June 22, 2023 11:14
@psss psss changed the title Add a single "tmpdir" fixture for all supported Python versions Add a single tmpdir fixture for all supported Python versions Jun 29, 2023
Copy link
Collaborator

@psss psss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just a few minor comments.

tests/unit/conftest.py Outdated Show resolved Hide resolved
tests/unit/test_file_require.py Outdated Show resolved Hide resolved
tests/unit/test_report_junit.py Outdated Show resolved Hide resolved
@happz happz force-pushed the unit-test-tmp-path-wrappers branch from 8075d7e to eb832ab Compare June 29, 2023 15:16
@psss psss self-assigned this Jun 29, 2023
See comments in `conftest.py`, but in general:

* because of our code, we would like to use `tmp_path` and `Path`
  it returns,
* but we have also our custom `tmt.utils.Path` we should be using
  instead of the original `pathlib.Path`,
* because of RHEL8, we also get pytest without `tmp_path` fixture,
* and `tmpdir` fixture returns `py.path.local`.

The patch adds our custom fixture which returns `tmt.utils.Path`
no matter what pytest we get. It's achieved by a test import,
trivial wrapper class, and two fixtures hiding the actual pytest
fixture involved.
@psss psss force-pushed the unit-test-tmp-path-wrappers branch from eb832ab to 4e05f95 Compare June 29, 2023 16:40
@psss psss merged commit 4e05f95 into main Jun 29, 2023
16 checks passed
@psss psss deleted the unit-test-tmp-path-wrappers branch June 29, 2023 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code style no functional change "No Functional Change" intended. Patch should not change tmt's behavior in any way.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants