Add Jenkins workflow and optimise testing mechanism #6400
Conversation
…needed code to save coverage per test
|
Thanks for your contribution. But this is not we change we would accept as this is clearly for a specific downstream implementation which is is not used in our repo. Also we are doing Coverage is a different way (but it is currently temporally disabled because of an upstream issue). I do like the approach of collecting coverage for each separate test. This is something I wanted to do myself in the past in a different project. The title does not describe the actual changes. Also this is not portable and more of a hack. Also you can achieve this without changing a single line of code. The compiler flags are supposed to be passed to Still some additional comments in case we will keep using this locally. |
| @@ -149,6 +149,11 @@ else ifeq ($(CXX), c++) | |||
| endif | |||
| endif | |||
|
|
|||
| ifeq ($(COVERAGE), 1) | |||
There was a problem hiding this comment.
This is file is generated via dmake so changes should be applied via the tool otherwise you might lose them.
| @@ -140,7 +140,7 @@ ifndef CXXFLAGS | |||
| endif | |||
|
|
|||
| ifeq (g++, $(findstring g++,$(CXX))) | |||
| override CXXFLAGS += -std=gnu++0x -pipe | |||
There was a problem hiding this comment.
Not sure about this change. Might be unnecessary to do that. We have some anachronisms in the file as well as some shortcomings. A bigger cleanup is pending review in #4968.
| const char* coverage = std::getenv("COVERAGE"); | ||
| if (coverage) { | ||
| std::string testDir = "./coverage_per_test/" + std::string(testname); // Adjust the path as needed | ||
| if (mkdir(testDir.c_str(), 0777) != 0) { |
There was a problem hiding this comment.
This is a big no-no. You should not create files or directory with execute or write permissions if not necessary. That might lead to security vulnerabilities. It should be 0644 at most.
| @@ -114,7 +141,7 @@ bool TestFixture::prepareTest(const char testname[]) | |||
| void TestFixture::teardownTest() | |||
| { | |||
| teardownTestInternal(); | |||
|
|
|||
| __gcov_flush(); | |||
There was a problem hiding this comment.
As this is done unconditionally it would interfere with other coverage executions. So this should be dependent on the environment you set up earlier.
|
No reply in over two weeks - closing. Please re-open if comments are being addressed and there is still something to apply. |
Add Jenkins workflow and optimise testing mechanism