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.

Sign up for GitHub

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

Jump to bottom

#4278 Added a CACHEDIR.TAG file to the cache directory #4504

Merged
merged 2 commits into from
Dec 11, 2018
Merged

#4278 Added a CACHEDIR.TAG file to the cache directory #4504

merged 2 commits into from
Dec 11, 2018

Conversation

feuillemorte
Copy link
Contributor

Fixes #4278

@codecov
Copy link

codecov bot commented Dec 4, 2018 •
edited
Loading

Codecov Report

Merging #4504 into features will increase coverage by <.01%.
The diff coverage is 91.66%.

Impacted file tree graph

@@             Coverage Diff              @@
##           features    #4504      +/-   ##
============================================
+ Coverage     95.75%   95.76%   +<.01%     
============================================
  Files           111      111              
  Lines         24867    24879      +12     
  Branches       2455     2456       +1     
============================================
+ Hits          23812    23825      +13     
+ Misses          746      745       -1     
  Partials        309      309
Flag Coverage Δ
#docs 29.38% <25%> (+0.07%) ⬆️
#doctesting 29.38% <25%> (+0.07%) ⬆️
#linting 29.38% <25%> (+0.07%) ⬆️
#linux 95.57% <91.66%> (-0.01%) ⬇️
#nobyte 92.46% <91.66%> (-0.01%) ⬇️
#numpy 93.2% <91.66%> (-0.01%) ⬇️
#pexpect 41.72% <25%> (-0.04%) ⬇️
#py27 93.8% <91.66%> (-0.01%) ⬇️
#py34 91.97% <91.66%> (+0.06%) ⬆️
#py35 91.99% <91.66%> (+0.06%) ⬆️
#py36 92.01% <91.66%> (+0.06%) ⬆️
#py37 93.94% <91.66%> (+0.02%) ⬆️
#trial 93.2% <91.66%> (-0.01%) ⬇️
#windows 94% <91.66%> (ø) ⬆️
#xdist 93.85% <91.66%> (-0.01%) ⬇️
Impacted Files Coverage Δ
testing/test_cacheprovider.py 99.72% <100%> (ø) ⬆️
src/_pytest/cacheprovider.py 96.69% <75%> (-0.42%) ⬇️
src/_pytest/capture.py 93.87% <0%> (+0.45%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5db46d2...d237197. Read the comment docs.

RonnyPfannschmidt
RonnyPfannschmidt approved these changes Dec 4, 2018
View reviewed changes
Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

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

well done thanks 👍

@nicoddemus
Copy link
Member

@feuillemorte glad to have you back btw 😁 Just curious what is the reason for this change?

@RonnyPfannschmidt
Copy link
Member

@nicoddemus that file is a marker for backup programs to skip the cache folders

@nicoddemus
Copy link
Member

Ahh right, thanks!

@nicoddemus
Copy link
Member

Waiting for @blueyed's approval. 👍

blueyed
blueyed approved these changes Dec 5, 2018
View reviewed changes
Copy link
Contributor

@blueyed blueyed left a comment

Choose a reason for hiding this comment

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

Great, thanks - adjusted the changelog a bit (mainly because it was missing punctuation).

I think we would not need an extra test for this, but at least it is not a slow one (using pytester).

testing/test_cacheprovider.py
cache = Cache.for_config(config)
cache.set("foo", "bar")
cachedir_tag_path = cache._cachedir.joinpath("CACHEDIR.TAG")
assert cachedir_tag_path.read_bytes() == CACHEDIR_TAG_CONTENT
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this could be just asserted in some existing test (to not increase number of tests unnecessarily).

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps mark the PR as "Changes requested" @blueyed? This makes clear that there are still issues to address here, both to @feuillemorte and by us (I almost merged this until I saw this comment).

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, it was just a comment/suggestion - feel free to merge it already.

@blueyed blueyed merged commit 0e4e8e0 into pytest-dev:features Dec 11, 2018
@feuillemorte feuillemorte deleted the 4278_add_cachedir_tag branch December 11, 2018 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicoddemus nicoddemus nicoddemus approved these changes

@blueyed blueyed blueyed approved these changes

@RonnyPfannschmidt RonnyPfannschmidt RonnyPfannschmidt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@feuillemorte @nicoddemus @RonnyPfannschmidt @blueyed