New PRs - tests and coverage

[Yegappan Lakshmanan]

Hi all,

When you submit a new PR, after the CI tests are successfully
completed, the coverage
information is shown in the PR diff page. Can you please go through
that and make sure
that most of the newly added or modified lines are covered by tests?
If not, please add
additional tests to make sure that the newly added lines are covered by tests.
In addition to the functional tests, also make sure to add tests for
invalid arguments
and negative/boundary conditions.

A lot of effort has gone in the last few years to add a significant
number of tests to cover
most of the Vim code base. Let us make sure that this doesn't regress.

The latest coverage information is available at
https://app.codecov.io/gh/vim/vim.

The instructions to generate the coverage information in a Linux system is at:
https://github.com/vim/vim/blob/master/src/Makefile#L668

Thanks,
Yegappan

[Christian Brabandt]

Thanks Yegappan,
that is true and I am trying to merge only when we have tests (even so
we could probably have more tests and I did not concentrate on the
coverage information).

However I noticed that the coverage information is a bit flaky,
sometimes it drops unexpectedly even when I am sure I added a tests just
for a specific patch. That is a bit annoying.

Let me try to be more careful when merging new patches.

Thanks,
Christian
--
Coding is easy; All you do is sit staring at a terminal until the drops
of blood form on your forehead.

[Yee Cheng Chin]

I think test coverage is good but I want to make sure we don't end up chasing the coverage % as an absolute goal (which just shows *line* coverage, not functionality coverage). I remember working on some PRs and rewriting some tests, and discovered the tests I was modifying didn't make too much sense. Looking through history, it seemed like they were mostly added to increase test coverage but didn't end up exercising the feature properly. It would run through the code which triggers the coverage, but didn't actually exercise the behavior in a way that verified the results or exercised the edge cases.

I guess what I'm saying is I absolutely think new functionality should be tested, but if some features are hard to test for the moment, we should probably leave it untested to make sure the lack of test coverage would tell us that. The test coverage % is there to give us a basic answer whether a line is exercised or not but we still need to make sure it's actually testing the edge cases, etc.

--
--
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

---
You received this message because you are subscribed to the Google Groups "vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to vim_dev+u...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/vim_dev/Zc8zMyBPrBeNjkpU%40256bit.org.