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. On Fri, Feb 16, 2024 at 2:04 AM Christian Brabandt <cbli...@256bit.org> wrote: > > On Do, 15 Feb 2024, Yegappan Lakshmanan wrote: > > > 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, > 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. > > -- > -- > 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+unsubscr...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/vim_dev/Zc8zMyBPrBeNjkpU%40256bit.org. > -- -- 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+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/vim_dev/CAHTeOx-c%2BY1wOXcE12vOSttYB6Y5Xh-AxwpMBAjqR9ykSyExXQ%40mail.gmail.com.
