Details on missing code coverage

Developers
1

I’ve got a PR that’s failure due to missing code coverage:

My question is; where are these coverage results pulled from?
It would make sense to add any additional tests alongside the existing ones, but I’m not sure if there are existing tests covering src/util-file.c.

If I need to create a new test, should I just add a new file into src/tests ? Do those tests link against everything?

Or should I use the UtRegisterTest() call which is used in various other util-*.c files?

Thanks,
Jeff

2

Coverage is currently taken from 2 CI jobs: one compiles with unittests and runs them, the other compiles w/o unittests and runs the Suricata-Verify tests. The results of both runs are submitted to Codecov.

For small functions we prefer unittests, for bigger tests, like checking if a feature leads to the correct log output, we prefer Suricata-Verify tests.

If you submit a Suricata-Verify PR, you can reference it in the Suricata PR by its number: suricata-verify-pr: NNN.

3

When you say to reference the suricata-verify PR, do you mean in commit comments?
I did that with one of my PRs (track TCP flags in first packet in each direction by rtkjweeks · Pull Request #5801 · OISF/suricata · GitHub) – an addition to the logging caused it to fail, so I adjusted the test so that it passed (Update tests for initial tcp flags capture by rtkjweeks · Pull Request #432 · OISF/suricata-verify · GitHub), and re-pushed the suricata PR with the suricata-verify-pr line in the commit message, but the tests failed in the same way (I was thinking the CI might pull that specific branch for testing to ensure the two work properly together, but if so, it seems like I’m doing it wrong; how should I be referencing the suricata-verify PR?

Thanks,
Jeff

4

@jweeks you can mention suricata-verify PR in the commit description. You’ll see in the PR template, we have,

#suricata-verify-pr:
#suricata-verify-repo:
#suricata-verify-branch:
#suricata-update-pr:
#suricata-update-repo:
#suricata-update-branch:
#libhtp-pr:
#libhtp-repo:
#libhtp-branch:

You can remove # before suricata-verify-pr and mention just the PR number there.
e.g.

suricata-verify-pr: 234
#suricata-verify-repo:
#suricata-verify-branch:
#suricata-update-pr:
#suricata-update-repo:
#suricata-update-branch:
#libhtp-pr:
#libhtp-repo:
#libhtp-branch:

For this to work, you should have made the suricata-verify PR before you make the suricata PR so the CI can pull that exact PR and run tests with it.
CI does not read commit messages so the PR number should not be mentioned in the commit.

Does this help?

6

Something seems off… I’m not done my UTs, but I wanted to push upstream to see what the coverage was, and I see this:

All but the first two annotations seem to be complaining that my tests don’t have test coverage… how can I prevent it from checking coverage of my test code??

7

Codecov is correct, the tests are still in the outdated style. New tests are branchless. See StreamTcpTest02 as an example. We’re slowly converting tests to conform to this new style.