Skip to content

Commit e6a6e24

Browse files
firewavefirewave
committed
added/moved testing of tool invocation to TestCppcheck 
1 parent 46c5c0c commit e6a6e24

File tree

7 files changed

+192
-151
lines changed

7 files changed

+192
-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: 186 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,19 @@
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"
3132

3233
#include <algorithm>
3334
#include <cstddef>
35+
#include <cstdlib>
3436
#include <list>
3537
#include <sstream>
3638
#include <string>
39+
#include <unordered_set>
40+
#include <utility>
3741
#include <vector>
3842

3943
#include <simplecpp.h>
@@ -66,7 +70,11 @@ class TestCppcheck : public TestFixture {
6670
void run() override {
6771
TEST_CASE(getErrorMessages);
6872
TEST_CASE(checkWithFile);
73+
TEST_CASE(checkWithFileWithTools);
74+
TEST_CASE(checkWithFileWithToolsNoCommand);
6975
TEST_CASE(checkWithFS);
76+
TEST_CASE(checkWithFSWithTools);
77+
TEST_CASE(checkWithFSWithToolsNoCommand);
7078
TEST_CASE(suppress_error_library);
7179
TEST_CASE(unique_errors);
7280
TEST_CASE(unique_errors_2);
@@ -114,49 +122,217 @@ class TestCppcheck : public TestFixture {
114122
ASSERT(foundMissingIncludeSystem);
115123
}
116124

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

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

139-
void checkWithFS() const
245+
void checkWithFileWithTools() {
246+
checkWithFileInternal(true);
247+
}
248+
249+
void checkWithFileWithToolsNoCommand() {
250+
checkWithFileInternal(true, true);
251+
}
252+
253+
void checkWithFSInternal(bool tools, bool nocmd = false) const
140254
{
255+
REDIRECT;
141256
ScopedFile file("test.cpp",
142257
"int main()\n"
143258
"{\n"
144259
" int i = *((int*)0);\n"
145260
" return 0;\n"
146261
"}");
147262

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

162338
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)