Skip to content
This repository has been archived by the owner on Jan 10, 2023. It is now read-only.

Allow paths with globs in -f #400

Merged
merged 1 commit into from
Jan 21, 2021
Merged

Allow paths with globs in -f #400

merged 1 commit into from
Jan 21, 2021

Conversation

llucax
Copy link
Contributor

@llucax llucax commented Jan 13, 2021

Purpose

Allowing -f path/*/glob to work as expected (find files as the shell would when using that sort of glob). This also provides a more uniform interface as most options dealing with specifying files can deal with it in the expected way (even using the -f option to specify exclusions, like -f !path/*/glob, work).

Notable Changes

Just use -path instead of -name as an option to find when -f is used for inclusions (as said before, for exclusions is already used).

Tests and Risks?

Tests are not provided as I didn't see any tests for any other -f use case, but maybe I just missed them? I looked in the tests/test file. I'm not familiar enough with shunit to add tests -f myself.

There is the risk of surprising some user that is writing some coverage report to some path that includes a literal * as part of the path and that have other files that will now match the glob and are not wanted in the codecov report upload.

So something like: -f path/*/glob in a project producing the following files:

path/to/glob
path/*/glob

Now will only find the file path/*/glob and with this change it will find path/to/glob too.

But this looks like a very unlikely scenario.

@codecov
Copy link

codecov bot commented Jan 13, 2021

Codecov Report

Merging #400 (cb3fcba) into master (cec3c92) will increase coverage by 1.71%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #400      +/-   ##
==========================================
+ Coverage   48.55%   50.27%   +1.71%     
==========================================
  Files           5        5              
  Lines        1423     1456      +33     
==========================================
+ Hits          691      732      +41     
+ Misses        732      724       -8     
Impacted Files Coverage Δ
codecov 61.30% <0.00%> (+1.78%) ⬆️

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 cec3c92...cb3fcba. Read the comment docs.

@llucax
Copy link
Contributor Author

llucax commented Jan 13, 2021

BTW this issue was already reported at least in the codecov github action repository: codecov/codecov-action#144

Most options dealing with specifying files can deal with
paths/with/*/globs correctly by using the -path find option instead of
-name, except for -f (and only for inclusions, exclusions already use
-path). It seems to make sense to allow globs in full paths too for
inclusions (globs are already supported, but they will match any file
recursively).
@thomasrockhu
Copy link
Contributor

Thanks @llucax

@thomasrockhu thomasrockhu merged commit 5cb01a5 into codecov:master Jan 21, 2021
@llucax llucax deleted the glob-f branch January 21, 2021 21:38
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.

2 participants