Skip to content

Commit 8dee551

Browse files
firewavefirewave
authored
added testing of clang-tidy invocation to executor tests / also some cleanups (#5514)
1 parent eb076d8 commit 8dee551

11 files changed

+231
-69
lines changed

‎Makefile

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -802,7 +802,7 @@ test/testpostfixoperator.o: test/testpostfixoperator.cpp lib/check.h lib/checkpo
802802
test/testpreprocessor.o: test/testpreprocessor.cpp externals/simplecpp/simplecpp.h lib/check.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/preprocessor.h lib/settings.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h test/helpers.h
803803
$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testpreprocessor.cpp
804804

805-
test/testprocessexecutor.o: test/testprocessexecutor.cpp cli/executor.h cli/processexecutor.h lib/check.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h test/helpers.h test/redirect.h
805+
test/testprocessexecutor.o: test/testprocessexecutor.cpp cli/executor.h cli/processexecutor.h lib/analyzerinfo.h lib/check.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h test/helpers.h test/redirect.h
806806
$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testprocessexecutor.cpp
807807

808808
test/testsettings.o: test/testsettings.cpp lib/check.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h test/fixture.h
@@ -841,7 +841,7 @@ test/testsuppressions.o: test/testsuppressions.cpp cli/cppcheckexecutor.h cli/ex
841841
test/testsymboldatabase.o: test/testsymboldatabase.cpp lib/check.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/sourcelocation.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h test/helpers.h
842842
$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testsymboldatabase.cpp
843843

844-
test/testthreadexecutor.o: test/testthreadexecutor.cpp cli/executor.h cli/threadexecutor.h lib/check.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h test/helpers.h test/redirect.h
844+
test/testthreadexecutor.o: test/testthreadexecutor.cpp cli/executor.h cli/threadexecutor.h lib/analyzerinfo.h lib/check.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h test/helpers.h test/redirect.h
845845
$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testthreadexecutor.cpp
846846

847847
test/testtimer.o: test/testtimer.cpp lib/check.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/timer.h lib/utils.h test/fixture.h

‎cli/cppcheckexecutor.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -325,9 +325,9 @@ int CppCheckExecutor::check_internal(CppCheck& cppcheck)
325325
returnValue = executor.check();
326326
} else {
327327
#if defined(THREADING_MODEL_THREAD)
328-
ThreadExecutor executor(mFiles, settings, settings.nomsg, *this);
328+
ThreadExecutor executor(mFiles, settings, settings.nomsg, *this, CppCheckExecutor::executeCommand);
329329
#elif defined(THREADING_MODEL_FORK)
330-
ProcessExecutor executor(mFiles, settings, settings.nomsg, *this);
330+
ProcessExecutor executor(mFiles, settings, settings.nomsg, *this, CppCheckExecutor::executeCommand);
331331
#endif
332332
returnValue = executor.check();
333333
}

‎cli/processexecutor.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,9 @@ enum class Color;
6262
using std::memset;
6363

6464

65-
ProcessExecutor::ProcessExecutor(const std::map<std::string, std::size_t> &files, const Settings &settings, Suppressions &suppressions, ErrorLogger &errorLogger)
65+
ProcessExecutor::ProcessExecutor(const std::map<std::string, std::size_t> &files, const Settings &settings, Suppressions &suppressions, ErrorLogger &errorLogger, CppCheck::ExecuteCmdFn executeCommand)
6666
: Executor(files, settings, suppressions, errorLogger)
67+
, mExecuteCommand(std::move(executeCommand))
6768
{
6869
assert(mSettings.jobs > 1);
6970
}
@@ -272,7 +273,7 @@ unsigned int ProcessExecutor::check()
272273
close(pipes[0]);
273274

274275
PipeWriter pipewriter(pipes[1]);
275-
CppCheck fileChecker(pipewriter, false, CppCheckExecutor::executeCommand);
276+
CppCheck fileChecker(pipewriter, false, mExecuteCommand);
276277
fileChecker.settings() = mSettings;
277278
unsigned int resultOfCheck = 0;
278279

‎cli/processexecutor.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#ifndef PROCESSEXECUTOR_H
2020
#define PROCESSEXECUTOR_H
2121

22+
#include "cppcheck.h"
2223
#include "executor.h"
2324

2425
#include <cstddef>
@@ -38,7 +39,7 @@ class Suppressions;
3839
*/
3940
class ProcessExecutor : public Executor {
4041
public:
41-
ProcessExecutor(const std::map<std::string, std::size_t> &files, const Settings &settings, Suppressions &suppressions, ErrorLogger &errorLogger);
42+
ProcessExecutor(const std::map<std::string, std::size_t> &files, const Settings &settings, Suppressions &suppressions, ErrorLogger &errorLogger, CppCheck::ExecuteCmdFn executeCommand);
4243
ProcessExecutor(const ProcessExecutor &) = delete;
4344
void operator=(const ProcessExecutor &) = delete;
4445

@@ -63,6 +64,8 @@ class ProcessExecutor : public Executor {
6364
* @param msg The error message
6465
*/
6566
void reportInternalChildErr(const std::string &childname, const std::string &msg);
67+
68+
CppCheck::ExecuteCmdFn mExecuteCommand;
6669
};
6770

6871
/// @}

‎cli/threadexecutor.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,9 @@
4141

4242
enum class Color;
4343

44-
ThreadExecutor::ThreadExecutor(const std::map<std::string, std::size_t> &files, const Settings &settings, Suppressions &suppressions, ErrorLogger &errorLogger)
44+
ThreadExecutor::ThreadExecutor(const std::map<std::string, std::size_t> &files, const Settings &settings, Suppressions &suppressions, ErrorLogger &errorLogger, CppCheck::ExecuteCmdFn executeCommand)
4545
: Executor(files, settings, suppressions, errorLogger)
46+
, mExecuteCommand(std::move(executeCommand))
4647
{
4748
assert(mSettings.jobs > 1);
4849
}
@@ -82,8 +83,8 @@ class SyncLogForwarder : public ErrorLogger
8283
class ThreadData
8384
{
8485
public:
85-
ThreadData(ThreadExecutor &threadExecutor, ErrorLogger &errorLogger, const Settings &settings, const std::map<std::string, std::size_t> &files, const std::list<ImportProject::FileSettings> &fileSettings)
86-
: mFiles(files), mFileSettings(fileSettings), mSettings(settings), logForwarder(threadExecutor, errorLogger)
86+
ThreadData(ThreadExecutor &threadExecutor, ErrorLogger &errorLogger, const Settings &settings, const std::map<std::string, std::size_t> &files, const std::list<ImportProject::FileSettings> &fileSettings, CppCheck::ExecuteCmdFn executeCommand)
87+
: mFiles(files), mFileSettings(fileSettings), mSettings(settings), mExecuteCommand(std::move(executeCommand)), logForwarder(threadExecutor, errorLogger)
8788
{
8889
mItNextFile = mFiles.begin();
8990
mItNextFileSettings = mFileSettings.begin();
@@ -115,7 +116,7 @@ class ThreadData
115116
}
116117

117118
unsigned int check(ErrorLogger &errorLogger, const std::string *file, const ImportProject::FileSettings *fs) const {
118-
CppCheck fileChecker(errorLogger, false, CppCheckExecutor::executeCommand);
119+
CppCheck fileChecker(errorLogger, false, mExecuteCommand);
119120
fileChecker.settings() = mSettings; // this is a copy
120121

121122
unsigned int result;
@@ -153,6 +154,7 @@ class ThreadData
153154

154155
std::mutex mFileSync;
155156
const Settings &mSettings;
157+
CppCheck::ExecuteCmdFn mExecuteCommand;
156158

157159
public:
158160
SyncLogForwarder logForwarder;
@@ -180,7 +182,7 @@ unsigned int ThreadExecutor::check()
180182
std::vector<std::future<unsigned int>> threadFutures;
181183
threadFutures.reserve(mSettings.jobs);
182184

183-
ThreadData data(*this, mErrorLogger, mSettings, mFiles, mSettings.project.fileSettings);
185+
ThreadData data(*this, mErrorLogger, mSettings, mFiles, mSettings.project.fileSettings, mExecuteCommand);
184186

185187
for (unsigned int i = 0; i < mSettings.jobs; ++i) {
186188
try {

‎cli/threadexecutor.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#ifndef THREADEXECUTOR_H
2020
#define THREADEXECUTOR_H
2121

22+
#include "cppcheck.h"
2223
#include "executor.h"
2324

2425
#include <cstddef>
@@ -37,14 +38,16 @@ class Suppressions;
3738
* all files using threads.
3839
*/
3940
class ThreadExecutor : public Executor {
41+
friend class SyncLogForwarder;
42+
4043
public:
41-
ThreadExecutor(const std::map<std::string, std::size_t> &files, const Settings &settings, Suppressions &suppressions, ErrorLogger &errorLogger);
44+
ThreadExecutor(const std::map<std::string, std::size_t> &files, const Settings &settings, Suppressions &suppressions, ErrorLogger &errorLogger, CppCheck::ExecuteCmdFn executeCommand);
4245
ThreadExecutor(const ThreadExecutor &) = delete;
4346
void operator=(const ThreadExecutor &) = delete;
4447

4548
unsigned int check() override;
4649

47-
friend class SyncLogForwarder;
50+
CppCheck::ExecuteCmdFn mExecuteCommand;
4851
};
4952

5053
/// @}

‎lib/library.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,8 @@ class CPPCHECKLIB Library {
5454
// TODO: get rid of this
5555
friend class TestSymbolDatabase; // For testing only
5656
friend class TestSingleExecutorBase; // For testing only
57-
friend class TestThreadExecutor; // For testing only
58-
friend class TestProcessExecutor; // For testing only
57+
friend class TestThreadExecutorBase; // For testing only
58+
friend class TestProcessExecutorBase; // For testing only
5959

6060
public:
6161
Library() = default;

‎test/testprocessexecutor.cpp

Lines changed: 92 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -34,15 +34,18 @@
3434
#include <utility>
3535
#include <vector>
3636

37-
class TestProcessExecutor : public TestFixture {
37+
class TestProcessExecutorBase : public TestFixture {
3838
public:
39-
TestProcessExecutor() : TestFixture("TestProcessExecutor") {}
39+
TestProcessExecutorBase(const char * const name, bool useFS) : TestFixture(name), useFS(useFS) {}
4040

4141
private:
4242
Settings settings = settingsBuilder().library("std.cfg").build();
43+
bool useFS;
4344

44-
static std::string fprefix()
45+
std::string fprefix() const
4546
{
47+
if (useFS)
48+
return "processfs";
4649
return "process";
4750
}
4851

@@ -53,6 +56,10 @@ class TestProcessExecutor : public TestFixture {
5356
SHOWTIME_MODES showtime = SHOWTIME_MODES::SHOWTIME_NONE;
5457
const char* plistOutput = nullptr;
5558
std::vector<std::string> filesList;
59+
bool clangTidy = false;
60+
bool executeCommandCalled = false;
61+
std::string exe;
62+
std::vector<std::string> args;
5663
};
5764

5865
/**
@@ -63,35 +70,67 @@ class TestProcessExecutor : public TestFixture {
6370
errout.str("");
6471
output.str("");
6572

73+
Settings s = settings;
74+
6675
std::map<std::string, std::size_t> filemap;
6776
if (opt.filesList.empty()) {
6877
for (int i = 1; i <= files; ++i) {
69-
std::ostringstream oss;
70-
oss << fprefix() << "_" << i << ".cpp";
71-
filemap[oss.str()] = data.size();
78+
std::string f_s = fprefix() + "_" + std::to_string(i) + ".cpp";
79+
filemap[f_s] = data.size();
80+
if (useFS) {
81+
ImportProject::FileSettings fs;
82+
fs.filename = std::move(f_s);
83+
s.project.fileSettings.emplace_back(std::move(fs));
84+
}
7285
}
7386
}
7487
else {
7588
for (const auto& f : opt.filesList)
7689
{
7790
filemap[f] = data.size();
91+
if (useFS) {
92+
ImportProject::FileSettings fs;
93+
fs.filename = f;
94+
s.project.fileSettings.emplace_back(std::move(fs));
95+
}
7896
}
7997
}
8098

81-
Settings s = settings;
8299
s.jobs = jobs;
83100
s.showtime = opt.showtime;
84-
settings.quiet = opt.quiet;
101+
s.quiet = opt.quiet;
85102
if (opt.plistOutput)
86103
s.plistOutput = opt.plistOutput;
87-
// TODO: test with settings.project.fileSettings;
88-
ProcessExecutor executor(filemap, s, s.nomsg, *this);
104+
105+
bool executeCommandCalled = false;
106+
std::string exe;
107+
std::vector<std::string> args;
108+
// NOLINTNEXTLINE(performance-unnecessary-value-param)
109+
auto executeFn = [&executeCommandCalled, &exe, &args](std::string e,std::vector<std::string> a,std::string,std::string&){
110+
executeCommandCalled = true;
111+
exe = std::move(e);
112+
args = std::move(a);
113+
return EXIT_SUCCESS;
114+
};
115+
89116
std::vector<std::unique_ptr<ScopedFile>> scopedfiles;
90117
scopedfiles.reserve(filemap.size());
91118
for (std::map<std::string, std::size_t>::const_iterator i = filemap.cbegin(); i != filemap.cend(); ++i)
92119
scopedfiles.emplace_back(new ScopedFile(i->first, data));
93120

121+
// clear files list so only fileSettings are used
122+
if (useFS)
123+
filemap.clear();
124+
125+
ProcessExecutor executor(filemap, s, s.nomsg, *this, executeFn);
94126
ASSERT_EQUALS(result, executor.check());
127+
ASSERT_EQUALS(opt.executeCommandCalled, executeCommandCalled);
128+
ASSERT_EQUALS(opt.exe, exe);
129+
ASSERT_EQUALS(opt.args.size(), args.size());
130+
for (int i = 0; i < args.size(); ++i)
131+
{
132+
ASSERT_EQUALS(opt.args[i], args[i]);
133+
}
95134
}
96135

97136
void run() override {
@@ -106,6 +145,7 @@ class TestProcessExecutor : public TestFixture {
106145
TEST_CASE(one_error_less_files);
107146
TEST_CASE(one_error_several_files);
108147
TEST_CASE(markup);
148+
TEST_CASE(clangTidy);
109149
TEST_CASE(showtime_top5_file);
110150
TEST_CASE(showtime_top5_summary);
111151
TEST_CASE(showtime_file);
@@ -148,15 +188,15 @@ class TestProcessExecutor : public TestFixture {
148188
}
149189

150190
void many_threads_plist() {
151-
const char plistOutput[] = "plist_process/";
191+
const std::string plistOutput = "plist_" + fprefix() + "/";
152192
ScopedFile plistFile("dummy", "", plistOutput);
153193

154194
check(16, 100, 100,
155195
"int main()\n"
156196
"{\n"
157197
" char *a = malloc(10);\n"
158198
" return 0;\n"
159-
"}", dinit(CheckOptions, $.plistOutput = plistOutput));
199+
"}", dinit(CheckOptions, $.plistOutput = plistOutput.c_str()));
160200
}
161201

162202
void no_errors_more_files() {
@@ -241,6 +281,34 @@ class TestProcessExecutor : public TestFixture {
241281
settings = settingsOld;
242282
}
243283

284+
void clangTidy() {
285+
// TODO: we currently only invoke it with ImportProject::FileSettings
286+
if (!useFS)
287+
return;
288+
289+
#ifdef _WIN32
290+
const char exe[] = "clang-tidy.exe";
291+
#else
292+
const char exe[] = "clang-tidy";
293+
#endif
294+
(void)exe;
295+
296+
const std::string file = fprefix() + "_1.cpp";
297+
// TODO: the invocation cannot be checked as the code is called in the forked process
298+
check(2, 1, 0,
299+
"int main()\n"
300+
"{\n"
301+
" return 0;\n"
302+
"}",
303+
dinit(CheckOptions,
304+
$.quiet = false,
305+
$.clangTidy = true /*,
306+
$.executeCommandCalled = true,
307+
$.exe = exe,
308+
$.args = {"-quiet", "-checks=*,-clang-analyzer-*,-llvm*", file, "--"}*/));
309+
ASSERT_EQUALS("Checking " + file + " ...\n", output.str());
310+
}
311+
244312
// TODO: provide data which actually shows values above 0
245313

246314
// TODO: should this be logged only once like summary?
@@ -303,8 +371,18 @@ class TestProcessExecutor : public TestFixture {
303371
TODO_ASSERT(output_s.find("Check time: " + fprefix() + "_2.cpp: ") != std::string::npos);
304372
}
305373

306-
// TODO: test clang-tidy
307374
// TODO: test whole program analysis
308375
};
309376

310-
REGISTER_TEST(TestProcessExecutor)
377+
class TestProcessExecutorFiles : public TestProcessExecutorBase {
378+
public:
379+
TestProcessExecutorFiles() : TestProcessExecutorBase("TestProcessExecutorFiles", false) {}
380+
};
381+
382+
class TestProcessExecutorFS : public TestProcessExecutorBase {
383+
public:
384+
TestProcessExecutorFS() : TestProcessExecutorBase("TestProcessExecutorFS", true) {}
385+
};
386+
387+
REGISTER_TEST(TestProcessExecutorFiles)
388+
REGISTER_TEST(TestProcessExecutorFS)

0 commit comments

Comments
 (0)