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

Code coverage reports with covimerage #1586

Merged
merged 2 commits into from
Dec 7, 2017
Merged

Code coverage reports with covimerage #1586

merged 2 commits into from
Dec 7, 2017

Conversation

arp242
Copy link
Contributor

@arp242 arp242 commented Nov 26, 2017

https://github.com/Vimjas/covimerage

Seems to work well: https://github.com/Carpetsmoker/vim-go/pull/3

You'll need to enable codecov.io on vim-go to get this to work here too @fatih.

Also thank you @blueyed for covimerage.

Copy link

@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.

Nice! You're welcome.

+'filetype plugin indent on' \
+'packloadall!' \
"$@"
fi
Copy link

Choose a reason for hiding this comment

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

Would be nice to have the base command just once, and then prepend the covimerage part in case of coverage=1 - might help with keeping it in sync.

You could use ${coverage:+covimerage -q run --report-file /tmp/vim-go-test/cov-profile.txt --append} - and then have $coverage be empty/unset without -c.

scripts/test Outdated
# Submit coverage reports
if [ -n "$coverage" ]; then
coverage xml --omit '*_test.vim'
codecov -X search gcov pycov -f coverage.xml --required
Copy link

Choose a reason for hiding this comment

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

You could use --flags here, too, e.g. --flags ${vim##*/} (not tested).

@blueyed
Copy link

blueyed commented Nov 26, 2017

scripts/test Outdated
# Submit coverage reports
if [ -n "$coverage" ]; then
coverage xml --omit '*_test.vim'
codecov -X search gcov pycov -f coverage.xml --required --flags "$vim"
Copy link

@blueyed blueyed Nov 27, 2017

Choose a reason for hiding this comment

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

You need to replace things, i.e. codecov_flags=${vim//,/}; codecov_flags=${codecov_flags//-/} and then use this with --flags $codecov_flags.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, I replace - and . now (sending vim80 instead of vim-8.0) and still doesn't seem to work. Not sure if this is a Codecov error. I've noticed that codecov can behave quirky if there is no report for master yet (for example, it keeps showing "Reports are queued for processing... Please review report with caution, it may change." for this PR).

Copy link

Choose a reason for hiding this comment

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

The replacement does not seem to work:

.query pr=1586&service=travis&package=py2.0.9&job=308161852&flags=vim-7.4&build=621.1&branch=master&commit=c26daeaa606e0abafadfe3efae3895b8bd898d11&slug=fatih%2Fvim-go

Copy link

Choose a reason for hiding this comment

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

..it uses &flags=vim-7.4.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

build=621.1 is the previous build though. For some reason Travis never started a build for the latest commit (which also explains why it doesn't show in Codecov). I pushed another commit and now it seems to work 👍

scripts/run-vim Outdated
"$@"
fi



Copy link

Choose a reason for hiding this comment

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

The /home/travis/gopath/src/github.com/fatih/vim-go/scripts/run-vim: 34: /home/travis/gopath/src/github.com/fatih/vim-go/scripts/run-vim: covimerage: not found here is really weird.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, not sure what's up with that either... It worked fine on my local machine.

Copy link

Choose a reason for hiding this comment

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

Maybe you have bash as /bin/sh, while Travis has not - or the other way around?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it's a link to bash; guess this invokes from bash-specific behaviour? Pff...

I think it's okay like this, for now. I have vague plans to rewrite this all to Go one day anyway. Shell scripting is nice to get started, but it very quickly becomes very unwieldy IMHO.

Copy link

Choose a reason for hiding this comment

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

Yeah..
I've noticed that your scripts are even affected by having CDPATH defined etc..
As for testing against different Vims you might want to consider https://github.com/tweekmonster/vim-testbed - I am using this with Neomake, and it is nice with CircleCI where jobs can be run in a specific Docker image right away.

I've just released covimerage 0.0.3 with the -l option.

@codecov-io
Copy link

codecov-io commented Nov 28, 2017

Codecov Report

❗ No coverage uploaded for pull request base (master@7f46735). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##             master   #1586   +/-   ##
========================================
  Coverage          ?   5.09%           
========================================
  Files             ?      53           
  Lines             ?    4123           
  Branches          ?       0           
========================================
  Hits              ?     210           
  Misses            ?    3913           
  Partials          ?       0
Flag Coverage Δ
#nvim 5.09% <ø> (?)

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 7f46735...e860c37. Read the comment docs.

+'packloadall!' \
"$@"
if [ $coverage -eq 1 ]; then
covimerage -q run --report-file /tmp/vim-go-test/cov-profile.txt --append \
Copy link

Choose a reason for hiding this comment

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

btw: why do you use -q?
I think not making it quiet should result in sensible output, and e.g. warnings in case some dict functions cannot be found etc - not sure if this suppressed with one -q only already.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As a general rule, I dislike programs that output a lot of informational messages. Basically, the "rule of silence" from The Art of Unix Programming.

Without the -q flag it adds a lot of output:

$ ./scripts/test -c vim-8.0
Running cmd: /tmp/vim-go-test/vim-8.0-install/bin/vim --noplugin -u NONE -N '+set shm+=WAFI rtp=/tmp/vim-go-test/vim-8.0-install/share/vim/vimgo packpath=/tmp/vim-go-test/vim-8.0-install/share/vim/vimgo,/home/martin/.vim/pack/plugins/start/vim-go' '+filetype plugin indent on' '+packloadall!' -e '+silent e /home/martin/.vim/pack/plugins/start/vim-go/autoload/go/tags_test.vim' '+let g:test_verbose=0' -S ./scripts/runtest.vim --cmd 'profile start /tmp/covimerage.profile.j5euwlyg' --cmd 'profile! file ./*' (in /home/martin/.vim/pack/plugins/start/vim-go)
Parsing profile file /tmp/covimerage.profile.j5euwlyg.
Writing coverage file .coverage.covimerage.
ok   tags_test.vim              0.171777s / 2 tests
Running cmd: /tmp/vim-go-test/vim-8.0-install/bin/vim --noplugin -u NONE -N '+set shm+=WAFI rtp=/tmp/vim-go-test/vim-8.0-install/share/vim/vimgo packpath=/tmp/vim-go-test/vim-8.0-install/share/vim/vimgo,/home/martin/.vim/pack/plugins/start/vim-go' '+filetype plugin indent on' '+packloadall!' -e '+silent e /home/martin/.vim/pack/plugins/start/vim-go/autoload/go/fillstruct_test.vim' '+let g:test_verbose=0' -S ./scripts/runtest.vim --cmd 'profile start /tmp/covimerage.profile.d2794fk1' --cmd 'profile! file ./*' (in /home/martin/.vim/pack/plugins/start/vim-go)
Parsing profile file /tmp/covimerage.profile.d2794fk1.
Writing coverage file .coverage.covimerage.
ok   fillstruct_test.vim        0.742317s / 1 tests
Running cmd: /tmp/vim-go-test/vim-8.0-install/bin/vim --noplugin -u NONE -N '+set shm+=WAFI rtp=/tmp/vim-go-test/vim-8.0-install/share/vim/vimgo packpath=/tmp/vim-go-test/vim-8.0-install/share/vim/vimgo,/home/martin/.vim/pack/plugins/start/vim-go' '+filetype plugin indent on' '+packloadall!' -e '+silent e /home/martin/.vim/pack/plugins/start/vim-go/autoload/go/tool_test.vim' '+let g:test_verbose=0' -S ./scripts/runtest.vim --cmd 'profile start /tmp/covimerage.profile.dy8bfk6j' --cmd 'profile! file ./*' (in /home/martin/.vim/pack/plugins/start/vim-go)
Parsing profile file /tmp/covimerage.profile.dy8bfk6j.
Writing coverage file .coverage.covimerage.
ok   tool_test.vim              0.025166s / 2 tests
Running cmd: /tmp/vim-go-test/vim-8.0-install/bin/vim --noplugin -u NONE -N '+set shm+=WAFI rtp=/tmp/vim-go-test/vim-8.0-install/share/vim/vimgo packpath=/tmp/vim-go-test/vim-8.0-install/share/vim/vimgo,/home/martin/.vim/pack/plugins/start/vim-go' '+filetype plugin indent on' '+packloadall!' -e '+silent e /home/martin/.vim/pack/plugins/start/vim-go/autoload/go/impl_test.vim' '+let g:test_verbose=0' -S ./scripts/runtest.vim --cmd 'profile start /tmp/covimerage.profile.rnk3t2wt' --cmd 'profile! file ./*' (in /home/martin/.vim/pack/plugins/start/vim-go)
Parsing profile file /tmp/covimerage.profile.rnk3t2wt.
Writing coverage file .coverage.covimerage.
ok   impl_test.vim              0.056763s / 2 tests
Running cmd: /tmp/vim-go-test/vim-8.0-install/bin/vim --noplugin -u NONE -N '+set shm+=WAFI rtp=/tmp/vim-go-test/vim-8.0-install/share/vim/vimgo packpath=/tmp/vim-go-test/vim-8.0-install/share/vim/vimgo,/home/martin/.vim/pack/plugins/start/vim-go' '+filetype plugin indent on' '+packloadall!' -e '+silent e /home/martin/.vim/pack/plugins/start/vim-go/autoload/go/def_test.vim' '+let g:test_verbose=0' -S ./scripts/runtest.vim --cmd 'profile start /tmp/covimerage.profile.dxp6b0f7' --cmd 'profile! file ./*' (in /home/martin/.vim/pack/plugins/start/vim-go)
Parsing profile file /tmp/covimerage.profile.dxp6b0f7.
Writing coverage file .coverage.covimerage.
ok   def_test.vim               0.007708s / 2 tests
Running cmd: /tmp/vim-go-test/vim-8.0-install/bin/vim --noplugin -u NONE -N '+set shm+=WAFI rtp=/tmp/vim-go-test/vim-8.0-install/share/vim/vimgo packpath=/tmp/vim-go-test/vim-8.0-install/share/vim/vimgo,/home/martin/.vim/pack/plugins/start/vim-go' '+filetype plugin indent on' '+packloadall!' -e '+silent e /home/martin/.vim/pack/plugins/start/vim-go/autoload/go/fmt_test.vim' '+let g:test_verbose=0' -S ./scripts/runtest.vim --cmd 'profile start /tmp/covimerage.profile.z1leipmj' --cmd 'profile! file ./*' (in /home/martin/.vim/pack/plugins/start/vim-go)
Parsing profile file /tmp/covimerage.profile.z1leipmj.
Writing coverage file .coverage.covimerage.
ok   fmt_test.vim               0.050436s / 3 tests
Running cmd: /tmp/vim-go-test/vim-8.0-install/bin/vim --noplugin -u NONE -N '+set shm+=WAFI rtp=/tmp/vim-go-test/vim-8.0-install/share/vim/vimgo packpath=/tmp/vim-go-test/vim-8.0-install/share/vim/vimgo,/home/martin/.vim/pack/plugins/start/vim-go' '+filetype plugin indent on' '+packloadall!' -e '+silent e /home/martin/.vim/pack/plugins/start/vim-go/test/gopath_test.vim' '+let g:test_verbose=0' -S ./scripts/runtest.vim --cmd 'profile start /tmp/covimerage.profile.rjn2eh0c' --cmd 'profile! file ./*' (in /home/martin/.vim/pack/plugins/start/vim-go)
Parsing profile file /tmp/covimerage.profile.rjn2eh0c.
Writing coverage file .coverage.covimerage.
ok   gopath_test.vim            0.294958s / 2 tests

All tests PASSED

We don't have a lot of tests (yet), but this should hopefully increase by a lot soon.

I know this is only run on the CI, but still – it makes it harder to see what failed, and what output is from the test/code, and what is from the tooling, especially for contributors who do not regularly contribute and are unfamiliar with the testing system.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as I can see logger.INFO is only used for a handful of informational messages, by the way: https://github.com/Vimjas/covimerage/search?utf8=%E2%9C%93&q=LOGGER.info&type=

So this should be fine, I think.

Copy link

@blueyed blueyed Nov 28, 2017

Choose a reason for hiding this comment

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

I see.
I think using warning as the default level would make sense then - but it would change the meaning of -q then.
I'll consider adding an option -l / --loglevel to give this explicitly.
Vimjas/covimerage#25

@blueyed
Copy link

blueyed commented Nov 29, 2017

Please remove the mentioning of my username from the commit msg before merging, otherwise I might end up being notified when people merge/push this commit to forks etc.

@blueyed
Copy link

blueyed commented Nov 29, 2017

Link to report (via commit, as a workaround until there is a base report): https://codecov.io/gh/fatih/vim-go/tree/c96b2f655ebd0dcd25b7f086c28c998905a8a827

@arp242 arp242 merged commit 6366c6e into fatih:master Dec 7, 2017
@arp242 arp242 deleted the covimerage branch April 8, 2018 02:12
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.

3 participants