Skip to content

Conversation

dpelle
Copy link
Member

@dpelle dpelle commented Oct 5, 2018

This PR adds a test for the resolve() function.
In particular, testing with symlink cycle was not covered
with tests according to codcov:

https://codecov.io/gh/vim/vim/src/2bc152ab53c4b01072edf6ec2ff61e504cb03cbe/src/evalfunc.c#L9657

I put some FIXME comments in the test where resolve() does not behave
as I expected.

I'll remove WIP in the title when CI shows enough code coverage.

@codecov-io
Copy link

codecov-io commented Oct 5, 2018

Codecov Report

Merging #3513 into master will increase coverage by 0.3%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #3513     +/-   ##
=========================================
+ Coverage   76.87%   77.18%   +0.3%     
=========================================
  Files          99       99             
  Lines      136451   139012   +2561     
=========================================
+ Hits       104896   107291   +2395     
- Misses      31555    31721    +166
Impacted Files Coverage Δ
src/xdiff/xemit.c 67.1% <0%> (-1.39%) ⬇️
src/if_python3.c 74.8% <0%> (-1.38%) ⬇️
src/if_python.c 76.55% <0%> (-1.31%) ⬇️
src/spellfile.c 73.97% <0%> (-1.06%) ⬇️
src/xdiff/xpatience.c 89.22% <0%> (-0.59%) ⬇️
src/gui_beval.c 55.02% <0%> (-0.34%) ⬇️
src/if_perl.xs 86.12% <0%> (-0.31%) ⬇️
src/netbeans.c 27.03% <0%> (-0.28%) ⬇️
src/libvterm/src/termscreen.c 58.97% <0%> (-0.25%) ⬇️
src/xdiff/xhistogram.c 83.62% <0%> (-0.21%) ⬇️
... and 74 more

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 2bc152a...fc81dfb. Read the comment docs.

@dpelle dpelle changed the title [WIP] resolve() was not tested with a symlink cycle resolve() was not tested with a symlink cycle Oct 6, 2018
@dpelle
Copy link
Member Author

dpelle commented Oct 6, 2018

A few lines in resolve() are still not covered.
I did not see how to cover them, but I think that the PR can be merged.

@brammool brammool closed this in 2610990 Oct 6, 2018
janlazo added a commit to janlazo/neovim that referenced this pull request Oct 6, 2018
Problem:    resolve() was not tested with a symlink cycle.
Solution:   Add a test. (Dominique Pelle, closes vim/vim#3513)
vim/vim@2610990
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants