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

GHA: Measure the code coverage also on MS-Windows #6795

Closed
wants to merge 4 commits into from

Conversation

k-takata
Copy link
Member

And some improvements to the GHA environment.

  • Add COVERAGE=yes|no option to Make_cyg_ming.mak.
  • Enable to measure the code coverage on MinGW HUGE (x86, x64) on GHA and upload the data to codecov.
    (This doesn't upload the data to coveralls. Not sure it is worth doing.)
    Unfortunately, this will drop the code coverage about 3%. :-(
  • Fix that some tests were not executed on GHA.
    To fix this, copy all the files in src/ (not only src/testdir/) to src2/, and fix some tests.
    (The tests for gvim.exe is run at src/testdir/, and the tests for vim.exe is run at src2/testdir/.)
  • Update the flaky tests list.
    Some tests are flaky on GHA. Test_BufWrite_lockmarks() and Test_bufunload_all() are added to the list.
    Actually, there are some more flaky tests on GHA, but at least these two tests are often failed.
    (I tried to fix them, but I couldn't. They don't reproduce on my local environment.)

@codecov
Copy link

codecov bot commented Aug 26, 2020

Codecov Report

Merging #6795 into master will decrease coverage by 2.68%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6795      +/-   ##
==========================================
- Coverage   88.51%   85.83%   -2.69%     
==========================================
  Files         147      159      +12     
  Lines      160762   173353   +12591     
==========================================
+ Hits       142305   148804    +6499     
- Misses      18457    24549    +6092     
Impacted Files Coverage Δ
src/libvterm/src/rect.h 0.00% <0.00%> (-96.56%) ⬇️
src/libvterm/src/state.c 51.26% <0.00%> (-38.73%) ⬇️
src/libvterm/include/vterm.h 0.00% <0.00%> (-37.50%) ⬇️
src/libvterm/src/keyboard.c 51.02% <0.00%> (-37.41%) ⬇️
src/libvterm/src/pen.c 51.27% <0.00%> (-33.39%) ⬇️
src/libvterm/src/encoding.c 45.09% <0.00%> (-27.18%) ⬇️
src/sound.c 67.26% <0.00%> (-22.74%) ⬇️
src/libvterm/src/vterm.c 48.88% <0.00%> (-17.78%) ⬇️
src/libvterm/src/parser.c 85.09% <0.00%> (-10.01%) ⬇️
src/libvterm/src/mouse.c 40.98% <0.00%> (-7.35%) ⬇️
... and 134 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 2e08661...869b75d. Read the comment docs.

@vim-ml
Copy link

vim-ml commented Aug 26, 2020 via email

@k-takata
Copy link
Member Author

Because the code coverage of the Windows-specific files like gui_w32.c, os_mswin.c, os_win32.c, etc. were not measured. The coverage of those files is lower than the average. So, they decrease the overall coverage. Of course, it doesn't mean that merging this PR decreases total covered lines. The number of covered lines are increased.

@vim-ml
Copy link

vim-ml commented Aug 26, 2020 via email

@brammool
Copy link
Contributor

If a line of code is tested under Linux, does it mean it will work under MS-Windows? I wonder if we should have two separate coverage projects: One for Linux and one for MS-Windows. Otherwise we cannot see what code is actually tested in the MS-Windows environment and what is tested in the Linux environment. Most tests will run equally on both platforms, but not everything. Having two projects may help discovering what we are not testing, while it won't really increase the effort for writing tests much.

@k-takata
Copy link
Member Author

I wonder if we should have two separate coverage projects: One for Linux and one for MS-Windows.

Yes, I also think of that, but I couldn't find the way yet. Need more investigation.
I extracted some part of this PR into #6798 so that it can be merged separately.

@k-takata k-takata added CI Travis, AppVeyor, GHA, ... platform-windows labels Aug 27, 2020
@k-takata
Copy link
Member Author

I enabled the Flags feature.
Here is the result: https://codecov.io/gh/k-takata/vim/tree/869b75d1808a52e9de6f20850aa3ae19ec7e6a17

The percentage of the coverage is still calculated from total of linux and windows.
But if you open an individual file's page, you will see two buttons with flag icons: "windows" and "linux".
https://codecov.io/gh/k-takata/vim/src/869b75d1808a52e9de6f20850aa3ae19ec7e6a17/src/os_w32dll.c
image
When the flag icon is pink, the coverage on that environment is shown, and when it is gray the coverage is not shown.

E.g.
The coverage on windows:
image

The coverage on linux:
image
You will see that the 4 lines are covered on windows, but not on linux.

Do you think this is acceptable?

I wonder if we should have two separate coverage projects:

Creating two separate codecov projects doesn't seem possible.

@brammool
Copy link
Contributor

For a larger file switching between Windows and Linux is very slow. Also, the UI doesn't give any feedback, thus it looks like it doesn't work.
I don't see the percentage change, thus this is only useful when working on tests, to see what is covered where. But it doesn't give any clue how much of a file is tested on either system. That's an essential statistic that is missing.
Anybody have an idea how to improve this? Should we ask the codecov maintainers?

@k-takata
Copy link
Member Author

Should we ask the codecov maintainers?

I think so.
From here? https://community.codecov.io/

k-takata added a commit to k-takata/vim that referenced this pull request Sep 3, 2020
Extracted from vim#6795.

This adds `COVERAGE=yes` option to Make_cyg_ming.mak.
This makes someone measure the code coverage easily on MS-Windows on a
local PC.
@k-takata
Copy link
Member Author

Close in favor of #9750.

@k-takata k-takata closed this Feb 12, 2022
@k-takata k-takata deleted the mingw-coverage branch February 12, 2022 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Travis, AppVeyor, GHA, ... platform-windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants