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

Fix file exists checker #44

Merged
merged 3 commits into from
Oct 17, 2020
Merged

Fix file exists checker #44

merged 3 commits into from
Oct 17, 2020

Conversation

mszostok
Copy link
Owner

@mszostok mszostok commented Oct 17, 2020

Description

The main problem was that the file exists checker was using Go's built-in filepath.Glob method which doesn't support double-asterisk patterns.

Additionally, with patterns like *.js we need to search for all JS files, not only on the first level but also in nested directories.

Based on made investigation this problem was fixed and additional test coverage was added to document that contract.

Related issue(s)

Fixes #42
Fixes #22

The main problem was that the file exists checker uses go's built in filepath globber, which doesn't support double-asterisk patterns (golang/go#11862).
Additionally, with patterns like `*.js` we need to search for all JS files not only on the first level but also in nested directories.

Based on made investigation this problem was fixed and additional test covereage was added to document that contract.
@mszostok mszostok added bug Something isn't working documentation labels Oct 17, 2020
@codecov
Copy link

codecov bot commented Oct 17, 2020

Codecov Report

Merging #44 into master will increase coverage by 6.49%.
The diff coverage is 67.64%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #44      +/-   ##
==========================================
+ Coverage   18.26%   24.76%   +6.49%     
==========================================
  Files          10       10              
  Lines         312      323      +11     
==========================================
+ Hits           57       80      +23     
+ Misses        252      239      -13     
- Partials        3        4       +1     
Impacted Files Coverage Δ
internal/check/not_owned_file.go 0.00% <0.00%> (ø)
internal/check/valid_owner.go 0.00% <0.00%> (ø)
internal/check/api.go 36.66% <75.00%> (+19.42%) ⬆️
internal/check/file_exists.go 77.27% <93.33%> (+77.27%) ⬆️
internal/check/duplicated_pattern.go 78.57% <100.00%> (ø)

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 cbda10c...1de2a20. Read the comment docs.

@mszostok mszostok merged commit 32b0e86 into master Oct 17, 2020
@mszostok mszostok deleted the fix-file-exist branch October 17, 2020 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Files checker does not support globstar patterns (**) Filepath Checker incorrect
1 participant