Skip to content
This repository was archived by the owner on Sep 2, 2024. It is now read-only.

Use the tmpdir fixture for tests that create files#995

Merged
olliesilvester merged 2 commits into
mainfrom
use_tmpdir_when_tests_create_files
Nov 27, 2023
Merged

Use the tmpdir fixture for tests that create files#995
olliesilvester merged 2 commits into
mainfrom
use_tmpdir_when_tests_create_files

Conversation

@olliesilvester

Copy link
Copy Markdown
Contributor

Fixes #994

To test:

  1. Run tests
  2. Check if change is useful/makes sense

@codecov

codecov Bot commented Nov 22, 2023

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (372887a) 93.67% compared to head (5e2067a) 93.49%.
Report is 1 commits behind head on main.

❗ Current head 5e2067a differs from pull request most recent head c201b10. Consider uploading reports for the commit c201b10 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #995      +/-   ##
==========================================
- Coverage   93.67%   93.49%   -0.19%     
==========================================
  Files          56       56              
  Lines        2704     2704              
==========================================
- Hits         2533     2528       -5     
- Misses        171      176       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@d-perl d-perl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I didn't know about this fixture, this does seem nice!

It doesn't seem like your temp_directory fixture is doing anything different to the builtin tmpdir? so I think we can just use that. Also in that case you can delete the .../test_data/scratch directory and .gitignore entries

@olliesilvester

Copy link
Copy Markdown
Contributor Author

thanks, didnt see it was in the gitignore

@d-perl d-perl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good, thanks!

@DominicOram

Copy link
Copy Markdown
Collaborator

Nit: Can you tidy up the PR name to make more sense to a user please?

@olliesilvester olliesilvester changed the title Merge pull request #991 from use_tmpdir_when_tests_create_files Use the tmpdir fixture for tests that create files Nov 27, 2023
@olliesilvester
olliesilvester merged commit 216a684 into main Nov 27, 2023
@olliesilvester
olliesilvester deleted the use_tmpdir_when_tests_create_files branch November 27, 2023 14:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use pytest's tmpdir fixture for tests which require creating files

3 participants