Codecov indicates coverage change when no source code is touched

[dkroenke]

Required information

Operating system: All compilers

Compiler version: All compilers

Observed result or behaviour: When Pull-Request are opened, the codecov still post a comment in the PR that the coverage was touched even when no codefiles are modified. Example: https://github.com/eclipse-iceoryx/iceoryx/pull/773/files

It seems that often the coverage involving smart_c.inl is flickering. The timing_tests are already uploaded in a separate codecov target but there are still flacky tests. It could be also a wrong configuration in codecov.

• [ ] Update Codecov Uploader in Github CI to V2 https://about.codecov.io/blog/codecov-uploader-deprecation-plan/ and https://github.com/marketplace/actions/codecov#%EF%B8%8F--deprecration-of-v1
• [ ] Ensure that there are no flacky tests in the output of gcov/lcov
• [ ] Investigate a possible misconfiguration in the codecov configuration

Expected result or behaviour: Code Coverage should only change when test files are modified.

Conditions where it occurred / Performed steps: Open a Pull-Request without modifying source code files.

[elBoberido]

@dkroenke only the tests marked as TIMING_TEST are in a separate target. There are far more tests relying on a timing. These might have a wider time margin than the ones from TIMING_TEST which don't make them sporadically fail but still might result in different code paths.

Although I proposed something like TIMING_TEST, I came to the conclusion that it's the wrong path. On the one hand we tend to just make it a TIMING_TEST instead of thinking about a better solution and on the other hand it might even hide real errors. If we have a component which is reliably fails every second execution, it will always pass the TIMING_TEST

[dkroenke]

@elBoberido That's indeed a good point, we need to isolate those "hidden" Timing Tests and either need to isolate or rewrite them (the latter is prefered).

[elfenpiff]

@dkroenke I think we need a brainstorming session here. Some timing tests need to remain timing tests, like for instance a call to sem_timedwait but those cases are hopefully rare.

For all the other test cases we need strategies on how to circumvent the timing test with a neat usage of semaphores, condition variables, or whatever. This should be documented for all the developers so that we have it in our toolbox. Or maybe we can even abstract it and move it into the test utils.

Sometimes we use timing tests to ensure that we are in the middle of a callback and that another callback runs successfully concurrently. Here maybe we can use also makecontext and context switches. For instance, the sigaction is based on that as well as racer peppi - maybe we can use it also in this context.

I suggest the following approach:

1. Collect Timing test use cases.
2. Find alternative approaches for timing tests use cases, document them or provide them as testutil in iceoryx utils.
3. If timing tests are remaining, refactor TIMING_TEST_F so that is more robust. Idea is that the user has no write access to the timeout T, it is preset by the test fixture. Whenever a test fails once the timeout T is increased (until an upper limit). The reruns of the timing tests N are not accessible by the user, they are internally defined (e.g. 10 for instance). A successful timing test than means: running a timing test N times without any failure at a specified timeout T. If it fails once for the timeout T the timeout is increased until either the test is successful or the upper limit is reached. @elBoberido would this be robust enough?

[elBoberido]

@elfenpiff ideally we would have a synthetic time but I guess that's not possible for all tests. At least the abstractions making the synthetic time possible would need testing with real time.

I'm not sure about the increased timeout since it makes the actual test parameter nontransparent. It might help keep the test time low, though. But if it runs 10 times, these gains are compensated.

I don't know of a good generic solution. Maybe we need to check what other projects are doing.