Skip to content

Commit 988764d

Browse files
firewavefirewave
committed
added/moved testing of tool invocation to TestCppcheck [skip ci]
1 parent 46c5c0c commit 988764d

File tree

7 files changed

+189
-151
lines changed

7 files changed

+189
-151
lines changed

‎Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -753,7 +753,7 @@ test/testcondition.o: test/testcondition.cpp lib/addoninfo.h lib/check.h lib/che
753753
test/testconstructors.o: test/testconstructors.cpp lib/addoninfo.h lib/check.h lib/checkclass.h lib/checkers.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/tokenize.h lib/tokenlist.h lib/utils.h test/fixture.h test/helpers.h
754754
$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testconstructors.cpp
755755

756-
test/testcppcheck.o: test/testcppcheck.cpp externals/simplecpp/simplecpp.h lib/addoninfo.h lib/check.h lib/checkers.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/filesettings.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/tokenize.h lib/tokenlist.h lib/utils.h test/fixture.h test/helpers.h
756+
test/testcppcheck.o: test/testcppcheck.cpp externals/simplecpp/simplecpp.h lib/addoninfo.h lib/check.h lib/checkers.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/filesettings.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/tokenize.h lib/tokenlist.h lib/utils.h test/fixture.h test/helpers.h test/redirect.h
757757
$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testcppcheck.cpp
758758

759759
test/testerrorlogger.o: test/testerrorlogger.cpp externals/tinyxml2/tinyxml2.h lib/addoninfo.h lib/check.h lib/checkers.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/xml.h test/fixture.h test/helpers.h

‎lib/cppcheck.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -444,7 +444,7 @@ static std::vector<picojson::value> executeAddon(const AddonInfo &addonInfo,
444444
else if (!defaultPythonExe.empty())
445445
pythonExe = cmdFileName(defaultPythonExe);
446446
else {
447-
// store in static variable so we only look this up once
447+
// store in static variable so we only look this up once - TODO: do not cache globally
448448
static const std::string detectedPythonExe = detectPython(executeCommand);
449449
if (detectedPythonExe.empty())
450450
throw InternalError(nullptr, "Failed to auto detect python");

‎test/redirect.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ class RedirectOutputError {
4343
std::cerr.rdbuf(_err.rdbuf()); // assign streambuf to cerr
4444
}
4545

46+
// TODO: if an assert failed previously this will mask that failure
4647
/** Revert cout and cerr behaviour */
4748
~RedirectOutputError() noexcept(false) {
4849
std::cout.rdbuf(_oldCout); // restore cout's original streambuf

‎test/testcppcheck.cpp

Lines changed: 183 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include "helpers.h"
2626
#include "path.h"
2727
#include "preprocessor.h"
28+
#include "redirect.h"
2829
#include "settings.h"
2930
#include "standards.h"
3031
#include "suppressions.h"
@@ -66,7 +67,11 @@ class TestCppcheck : public TestFixture {
6667
void run() override {
6768
TEST_CASE(getErrorMessages);
6869
TEST_CASE(checkWithFile);
70+
TEST_CASE(checkWithFileWithTools);
71+
TEST_CASE(checkWithFileWithToolsNoCommand);
6972
TEST_CASE(checkWithFS);
73+
TEST_CASE(checkWithFSWithTools);
74+
TEST_CASE(checkWithFSWithToolsNoCommand);
7075
TEST_CASE(suppress_error_library);
7176
TEST_CASE(unique_errors);
7277
TEST_CASE(unique_errors_2);
@@ -114,49 +119,217 @@ class TestCppcheck : public TestFixture {
114119
ASSERT(foundMissingIncludeSystem);
115120
}
116121

117-
void checkWithFile() const
122+
static std::string exename(std::string exe)
118123
{
124+
#ifdef _WIN32
125+
return exe + ".exe";
126+
#else
127+
return exe;
128+
#endif
129+
}
130+
131+
CppCheck::ExecuteCmdFn getExecuteCommand(int& called) const
132+
{
133+
// NOLINTNEXTLINE(performance-unnecessary-value-param)
134+
return [&](std::string exe, std::vector<std::string> args, std::string redirect, std::string& /*output*/) -> int {
135+
++called;
136+
if (exe == exename("clang-tidy"))
137+
{
138+
ASSERT_EQUALS(4, args.size());
139+
ASSERT_EQUALS("-quiet", args[0]);
140+
ASSERT_EQUALS("-checks=*,-clang-analyzer-*,-llvm*", args[1]);
141+
ASSERT_EQUALS("test.cpp", args[2]);
142+
ASSERT_EQUALS("--", args[3]);
143+
ASSERT_EQUALS("2>&1", redirect);
144+
return EXIT_SUCCESS;
145+
}
146+
if (exe == exename("python3"))
147+
{
148+
ASSERT_EQUALS(1, args.size());
149+
ASSERT_EQUALS("--version", args[0]);
150+
ASSERT_EQUALS("2>&1", redirect);
151+
return EXIT_SUCCESS;
152+
}
153+
if (exe == exename("python"))
154+
{
155+
ASSERT_EQUALS(1, args.size());
156+
ASSERT_EQUALS("--version", args[0]);
157+
ASSERT_EQUALS("2>&1", redirect);
158+
return EXIT_SUCCESS;
159+
}
160+
ASSERT_MSG(false, "unhandled exe: " + exe);
161+
return EXIT_FAILURE;
162+
};
163+
}
164+
165+
void checkWithFileInternal(bool tools, bool nocmd = false) const
166+
{
167+
REDIRECT;
119168
ScopedFile file("test.cpp",
120169
"int main()\n"
121170
"{\n"
122171
" int i = *((int*)0);\n"
123172
" return 0;\n"
124173
"}");
125174

126-
const auto s = dinit(Settings, $.templateFormat = templateFormat);
175+
int called = 0;
176+
std::unordered_set<std::string> addons;
177+
std::vector<AddonInfo> addonInfo;
178+
if (tools)
179+
{
180+
addons.emplace("testcppcheck");
181+
addonInfo.emplace_back(AddonInfo());
182+
}
183+
const auto s = dinit(Settings,
184+
$.templateFormat = templateFormat,
185+
$.clangTidy = tools,
186+
$.addons = std::move (addons),
187+
$.addonInfos = std::move (addonInfo));
127188
Suppressions supprs;
128189
ErrorLogger2 errorLogger;
129-
CppCheck cppcheck(s, supprs, errorLogger, false, {});
190+
CppCheck::ExecuteCmdFn f;
191+
if (tools && !nocmd) {
192+
f = getExecuteCommand(called);
193+
}
194+
CppCheck cppcheck(s, supprs, errorLogger, false, f);
130195
ASSERT_EQUALS(1, cppcheck.check(FileWithDetails(file.path(), Path::identify(file.path(), false), 0)));
131196
// TODO: how to properly disable these warnings?
132197
errorLogger.ids.erase(std::remove_if(errorLogger.ids.begin(), errorLogger.ids.end(), [](const std::string& id) {
133198
return id == "logChecker";
134199
}), errorLogger.ids.end());
135-
ASSERT_EQUALS(1, errorLogger.ids.size());
136-
ASSERT_EQUALS("nullPointer", *errorLogger.ids.cbegin());
200+
errorLogger.errmsgs.erase(std::remove_if(errorLogger.errmsgs.begin(), errorLogger.errmsgs.end(), [](const ErrorMessage& msg) {
201+
return msg.id == "logChecker";
202+
}), errorLogger.errmsgs.end());
203+
if (tools)
204+
{
205+
ASSERT_EQUALS(2, errorLogger.ids.size());
206+
auto it = errorLogger.errmsgs.cbegin();
207+
ASSERT_EQUALS("nullPointer", it->id);
208+
++it;
209+
210+
if (nocmd)
211+
{
212+
ASSERT_EQUALS("internalError", it->id);
213+
ASSERT_EQUALS("Bailing out from analysis: Checking file failed: Failed to execute addon - no command callback provided", it->shortMessage()); // TODO: add addon name
214+
215+
// TODO: clang-tidy is currently not invoked for file inputs - see #12053
216+
// TODO: needs to become a proper error
217+
TODO_ASSERT_EQUALS("Failed to execute '" + exename("clang-tidy") + "' (no command callback provided)\n", "", GET_REDIRECT_ERROUT);
218+
219+
ASSERT_EQUALS(0, called); // not called because we check if the callback exists
220+
}
221+
else
222+
{
223+
ASSERT_EQUALS("internalError", it->id);
224+
ASSERT_EQUALS("Bailing out from analysis: Checking file failed: Failed to auto detect python", it->shortMessage()); // TODO: clarify what python is used for
225+
226+
// TODO: we cannot check this because the python detection is cached globally so this result will different dependent on how the test is called
227+
//ASSERT_EQUALS(2, called);
228+
}
229+
}
230+
else
231+
{
232+
ASSERT_EQUALS(0, called);
233+
ASSERT_EQUALS(1, errorLogger.ids.size());
234+
ASSERT_EQUALS("nullPointer", *errorLogger.ids.cbegin());
235+
}
236+
}
237+
238+
void checkWithFile() {
239+
checkWithFileInternal(false);
137240
}
138241

139-
void checkWithFS() const
242+
void checkWithFileWithTools() {
243+
checkWithFileInternal(true);
244+
}
245+
246+
void checkWithFileWithToolsNoCommand() {
247+
checkWithFileInternal(true, true);
248+
}
249+
250+
void checkWithFSInternal(bool tools, bool nocmd = false) const
140251
{
252+
REDIRECT;
141253
ScopedFile file("test.cpp",
142254
"int main()\n"
143255
"{\n"
144256
" int i = *((int*)0);\n"
145257
" return 0;\n"
146258
"}");
147259

148-
const auto s = dinit(Settings, $.templateFormat = templateFormat);
260+
int called = 0;
261+
std::unordered_set<std::string> addons;
262+
std::vector<AddonInfo> addonInfo;
263+
if (tools)
264+
{
265+
addons.emplace("testcppcheck");
266+
addonInfo.emplace_back(AddonInfo());
267+
}
268+
const auto s = dinit(Settings,
269+
$.templateFormat = templateFormat,
270+
$.clangTidy = tools,
271+
$.addons = std::move (addons),
272+
$.addonInfos = std::move (addonInfo));
149273
Suppressions supprs;
150274
ErrorLogger2 errorLogger;
151-
CppCheck cppcheck(s, supprs, errorLogger, false, {});
275+
CppCheck::ExecuteCmdFn f;
276+
if (tools && !nocmd) {
277+
f = getExecuteCommand(called);
278+
}
279+
CppCheck cppcheck(s, supprs, errorLogger, false, f);
152280
FileSettings fs{file.path(), Path::identify(file.path(), false), 0};
153281
ASSERT_EQUALS(1, cppcheck.check(fs));
154282
// TODO: how to properly disable these warnings?
155283
errorLogger.ids.erase(std::remove_if(errorLogger.ids.begin(), errorLogger.ids.end(), [](const std::string& id) {
156284
return id == "logChecker";
157285
}), errorLogger.ids.end());
158-
ASSERT_EQUALS(1, errorLogger.ids.size());
159-
ASSERT_EQUALS("nullPointer", *errorLogger.ids.cbegin());
286+
errorLogger.errmsgs.erase(std::remove_if(errorLogger.errmsgs.begin(), errorLogger.errmsgs.end(), [](const ErrorMessage& msg) {
287+
return msg.id == "logChecker";
288+
}), errorLogger.errmsgs.end());
289+
if (tools)
290+
{
291+
ASSERT_EQUALS(2, errorLogger.ids.size());
292+
auto it = errorLogger.errmsgs.cbegin();
293+
ASSERT_EQUALS("nullPointer", it->id);
294+
++it;
295+
296+
if (nocmd)
297+
{
298+
ASSERT_EQUALS("internalError", it->id);
299+
ASSERT_EQUALS("Bailing out from analysis: Checking file failed: Failed to execute addon - no command callback provided", it->shortMessage()); // TODO: add addon name
300+
301+
// TODO: needs to become a proper error
302+
ASSERT_EQUALS("Failed to execute '" + exename("clang-tidy") + "' (no command callback provided)\n", GET_REDIRECT_ERROUT);
303+
304+
ASSERT_EQUALS(0, called); // not called because we check if the callback exists
305+
}
306+
else
307+
{
308+
ASSERT_EQUALS("internalError", it->id);
309+
ASSERT_EQUALS("Bailing out from analysis: Checking file failed: Failed to auto detect python", it->shortMessage()); // TODO: clarify what python is used for
310+
311+
// TODO: we cannot check this because the python detection is cached globally so this result will different dependent on how the test is called
312+
//ASSERT_EQUALS(3, called);
313+
}
314+
}
315+
else
316+
{
317+
ASSERT_EQUALS(0, called);
318+
ASSERT_EQUALS(1, errorLogger.ids.size());
319+
ASSERT_EQUALS("nullPointer", *errorLogger.ids.cbegin());
320+
}
321+
}
322+
323+
void checkWithFS() {
324+
checkWithFSInternal(false);
325+
}
326+
327+
void checkWithFSWithTools() {
328+
checkWithFSInternal(true);
329+
}
330+
331+
void checkWithFSWithToolsNoCommand() {
332+
checkWithFSInternal(true, true);
160333
}
161334

162335
void suppress_error_library() const

‎test/testprocessexecutor.cpp

Lines changed: 1 addition & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,6 @@ class TestProcessExecutorBase : public TestFixture {
5656
SHOWTIME_MODES showtime = SHOWTIME_MODES::SHOWTIME_NONE;
5757
const char* plistOutput = nullptr;
5858
std::vector<std::string> filesList;
59-
bool clangTidy = false;
60-
bool executeCommandCalled = false;
61-
std::string exe;
62-
std::vector<std::string> args;
6359
};
6460

6561
/**
@@ -98,14 +94,8 @@ class TestProcessExecutorBase : public TestFixture {
9894
s.templateFormat = "{callstack}: ({severity}) {inconclusive:inconclusive: }{message}";
9995
Suppressions supprs;
10096

101-
bool executeCommandCalled = false;
102-
std::string exe;
103-
std::vector<std::string> args;
10497
// NOLINTNEXTLINE(performance-unnecessary-value-param)
105-
auto executeFn = [&executeCommandCalled, &exe, &args](std::string e,std::vector<std::string> a,std::string,std::string&){
106-
executeCommandCalled = true;
107-
exe = std::move(e);
108-
args = std::move(a);
98+
auto executeFn = [](std::string,std::vector<std::string>,std::string,std::string&){
10999
return EXIT_SUCCESS;
110100
};
111101

@@ -120,13 +110,6 @@ class TestProcessExecutorBase : public TestFixture {
120110

121111
ProcessExecutor executor(filelist, fileSettings, s, supprs, *this, executeFn);
122112
ASSERT_EQUALS(result, executor.check());
123-
ASSERT_EQUALS(opt.executeCommandCalled, executeCommandCalled);
124-
ASSERT_EQUALS(opt.exe, exe);
125-
ASSERT_EQUALS(opt.args.size(), args.size());
126-
for (std::size_t i = 0; i < args.size(); ++i)
127-
{
128-
ASSERT_EQUALS(opt.args[i], args[i]);
129-
}
130113
}
131114

132115
void run() override {
@@ -141,7 +124,6 @@ class TestProcessExecutorBase : public TestFixture {
141124
TEST_CASE(no_errors_equal_amount_files);
142125
TEST_CASE(one_error_less_files);
143126
TEST_CASE(one_error_several_files);
144-
TEST_CASE(clangTidy);
145127
TEST_CASE(showtime_top5_file);
146128
TEST_CASE(showtime_top5_summary);
147129
TEST_CASE(showtime_file);
@@ -250,34 +232,6 @@ class TestProcessExecutorBase : public TestFixture {
250232
ASSERT_EQUALS(num_files, cppcheck::count_all_of(errout_str(), "(error) Null pointer dereference: (int*)0"));
251233
}
252234

253-
void clangTidy() {
254-
// TODO: we currently only invoke it with ImportProject::FileSettings
255-
if (!useFS)
256-
return;
257-
258-
#ifdef _WIN32
259-
constexpr char exe[] = "clang-tidy.exe";
260-
#else
261-
constexpr char exe[] = "clang-tidy";
262-
#endif
263-
(void)exe;
264-
265-
const std::string file = fprefix() + "_1.cpp";
266-
// TODO: the invocation cannot be checked as the code is called in the forked process
267-
check(2, 1, 0,
268-
"int main()\n"
269-
"{\n"
270-
" return 0;\n"
271-
"}",
272-
dinit(CheckOptions,
273-
$.quiet = false,
274-
$.clangTidy = true /*,
275-
$.executeCommandCalled = true,
276-
$.exe = exe,
277-
$.args = {"-quiet", "-checks=*,-clang-analyzer-*,-llvm*", file, "--"}*/));
278-
ASSERT_EQUALS("Checking " + file + " ...\n", output_str());
279-
}
280-
281235
// TODO: provide data which actually shows values above 0
282236

283237
// TODO: should this be logged only once like summary?

0 commit comments

Comments
 (0)