Skip to content

Add Jenkins workflow and optimise testing mechanism #6400

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jboskovic wants to merge 1 commit into from
Closed

Add Jenkins workflow and optimise testing mechanism #6400

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ ifndef CXXFLAGS
endif

ifeq (g++, $(findstring g++,$(CXX)))
override CXXFLAGS += -std=gnu++0x -pipe

@firewave firewave May 12, 2024

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

CXXFLAGS += -std=gnu++0x -pipe
else ifeq (clang++, $(findstring clang++,$(CXX)))
override CXXFLAGS += -std=c++0x
else ifeq ($(CXX), c++)
Expand All @@ -149,6 +149,11 @@ else ifeq ($(CXX), c++)
endif
endif

ifeq ($(COVERAGE), 1)

@firewave firewave May 12, 2024

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is file is generated via dmake so changes should be applied via the tool otherwise you might lose them.

$(info Adding coverage flags to CXXFLAGS)
CXXFLAGS += -fprofile-arcs -ftest-coverage
endif

ifeq ($(HAVE_RULES),yes)
PCRE_CONFIG = $(shell which pcre-config)
ifeq ($(PCRE_CONFIG),)
Expand Down
29 changes: 28 additions & 1 deletion test/fixture.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@

#include "xml.h"

#include <cstdlib> // for getenv and setenv
#include <sys/stat.h> // for mkdir
extern "C" void __gcov_flush();

/**
* TestRegistry
**/
Expand Down Expand Up @@ -88,6 +92,29 @@ TestFixture::TestFixture(const char * const _name)

bool TestFixture::prepareTest(const char testname[])
{
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) {

@firewave firewave May 12, 2024

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

// If directory creation fails and it's not because the directory exists
if (errno != EEXIST) {
std::cerr << "Failed to create directory for test: " << testDir
<< ", Error: " << strerror(errno) << std::endl;
return false;
}
}

// Set the environment variables to point to the new directory
if (setenv("GCOV_PREFIX", testDir.c_str(), 1) != 0) {
std::cerr << "Failed to set GCOV_PREFIX environment variable." << std::endl;
return false;
}

if (setenv("GCOV_PREFIX_STRIP", "0", 1) != 0) {
std::cerr << "Failed to set GCOV_PREFIX_STRIP environment variable." << std::endl;
return false;
}
}
mVerbose = false;
mTemplateFormat.clear();
mTemplateLocation.clear();
Expand All @@ -114,7 +141,7 @@ bool TestFixture::prepareTest(const char testname[])
void TestFixture::teardownTest()
{
teardownTestInternal();

__gcov_flush();

@firewave firewave May 12, 2024

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this is done unconditionally it would interfere with other coverage executions. So this should be dependent on the environment you set up earlier.

{
const std::string s = errout_str();
if (!s.empty())
Expand Down