Skip to content

Commit b0cde34

Browse files
firewavefirewave
authored
refs #12167 - moved ordering of markup files into shared code / removed related test cases from executor tests (#5642)
This is not completely fixing the issue yet. `test-qml.py` still fails when using multiple threads.
1 parent d24074f commit b0cde34

8 files changed

+122
-172
lines changed

‎cli/cppcheckexecutor.cpp

+34-14
Original file line numberDiff line numberDiff line change
@@ -186,30 +186,40 @@ bool CppCheckExecutor::parseFromArgs(Settings &settings, int argc, const char* c
186186
logger.printMessage("Please use --suppress for ignoring results from the header files.");
187187
}
188188

189-
const std::vector<std::string>& pathnames = parser.getPathNames();
190-
const std::list<FileSettings>& fileSettings = parser.getFileSettings();
189+
const std::vector<std::string>& pathnamesRef = parser.getPathNames();
190+
const std::list<FileSettings>& fileSettingsRef = parser.getFileSettings();
191191

192192
// the inputs can only be used exclusively - CmdLineParser should already handle this
193-
assert(!(!pathnames.empty() && !fileSettings.empty()));
193+
assert(!(!pathnamesRef.empty() && !fileSettingsRef.empty()));
194194

195-
if (!fileSettings.empty()) {
195+
if (!fileSettingsRef.empty()) {
196+
std::list<FileSettings> fileSettings;
196197
if (!settings.fileFilters.empty()) {
197198
// filter only for the selected filenames from all project files
198-
std::copy_if(fileSettings.cbegin(), fileSettings.cend(), std::back_inserter(mFileSettings), [&](const FileSettings &fs) {
199+
std::copy_if(fileSettingsRef.cbegin(), fileSettingsRef.cend(), std::back_inserter(fileSettings), [&](const FileSettings &fs) {
199200
return matchglobs(settings.fileFilters, fs.filename);
200201
});
201-
if (mFileSettings.empty()) {
202+
if (fileSettings.empty()) {
202203
logger.printError("could not find any files matching the filter.");
203204
return false;
204205
}
205206
}
206207
else {
207-
mFileSettings = fileSettings;
208+
fileSettings = fileSettingsRef;
208209
}
210+
211+
// sort the markup last
212+
std::copy_if(fileSettings.cbegin(), fileSettings.cend(), std::back_inserter(mFileSettings), [&](const FileSettings &fs) {
213+
return !settings.library.markupFile(fs.filename) || !settings.library.processMarkupAfterCode(fs.filename);
214+
});
215+
216+
std::copy_if(fileSettings.cbegin(), fileSettings.cend(), std::back_inserter(mFileSettings), [&](const FileSettings &fs) {
217+
return settings.library.markupFile(fs.filename) && settings.library.processMarkupAfterCode(fs.filename);
218+
});
209219
}
210220

211-
if (!pathnames.empty()) {
212-
std::list<std::pair<std::string, std::size_t>> files;
221+
if (!pathnamesRef.empty()) {
222+
std::list<std::pair<std::string, std::size_t>> filesResolved;
213223
// TODO: this needs to be inlined into PathMatch as it depends on the underlying filesystem
214224
#if defined(_WIN32)
215225
// For Windows we want case-insensitive path matching
@@ -219,26 +229,36 @@ bool CppCheckExecutor::parseFromArgs(Settings &settings, int argc, const char* c
219229
#endif
220230
// Execute recursiveAddFiles() to each given file parameter
221231
const PathMatch matcher(ignored, caseSensitive);
222-
for (const std::string &pathname : pathnames) {
223-
const std::string err = FileLister::recursiveAddFiles(files, Path::toNativeSeparators(pathname), settings.library.markupExtensions(), matcher);
232+
for (const std::string &pathname : pathnamesRef) {
233+
const std::string err = FileLister::recursiveAddFiles(filesResolved, Path::toNativeSeparators(pathname), settings.library.markupExtensions(), matcher);
224234
if (!err.empty()) {
225235
// TODO: bail out?
226236
logger.printMessage(err);
227237
}
228238
}
229239

240+
std::list<std::pair<std::string, std::size_t>> files;
230241
if (!settings.fileFilters.empty()) {
231-
std::copy_if(files.cbegin(), files.cend(), std::inserter(mFiles, mFiles.end()), [&](const decltype(files)::value_type& entry) {
242+
std::copy_if(filesResolved.cbegin(), filesResolved.cend(), std::inserter(files, files.end()), [&](const decltype(filesResolved)::value_type& entry) {
232243
return matchglobs(settings.fileFilters, entry.first);
233244
});
234-
if (mFiles.empty()) {
245+
if (files.empty()) {
235246
logger.printError("could not find any files matching the filter.");
236247
return false;
237248
}
238249
}
239250
else {
240-
mFiles = files;
251+
files = std::move(filesResolved);
241252
}
253+
254+
// sort the markup last
255+
std::copy_if(files.cbegin(), files.cend(), std::inserter(mFiles, mFiles.end()), [&](const decltype(files)::value_type& entry) {
256+
return !settings.library.markupFile(entry.first) || !settings.library.processMarkupAfterCode(entry.first);
257+
});
258+
259+
std::copy_if(files.cbegin(), files.cend(), std::inserter(mFiles, mFiles.end()), [&](const decltype(files)::value_type& entry) {
260+
return settings.library.markupFile(entry.first) && settings.library.processMarkupAfterCode(entry.first);
261+
});
242262
}
243263

244264
if (mFiles.empty() && mFileSettings.empty()) {

‎cli/singleexecutor.cpp

+12-41
Original file line numberDiff line numberDiff line change
@@ -51,52 +51,23 @@ unsigned int SingleExecutor::check()
5151
unsigned int c = 0;
5252

5353
for (std::list<std::pair<std::string, std::size_t>>::const_iterator i = mFiles.cbegin(); i != mFiles.cend(); ++i) {
54-
if (!mSettings.library.markupFile(i->first) || !mSettings.library.processMarkupAfterCode(i->first)) {
55-
result += mCppcheck.check(i->first);
56-
processedsize += i->second;
57-
++c;
58-
if (!mSettings.quiet)
59-
reportStatus(c, mFiles.size(), processedsize, totalfilesize);
60-
// TODO: call analyseClangTidy()?
61-
}
54+
result += mCppcheck.check(i->first);
55+
processedsize += i->second;
56+
++c;
57+
if (!mSettings.quiet)
58+
reportStatus(c, mFiles.size(), processedsize, totalfilesize);
59+
// TODO: call analyseClangTidy()?
6260
}
6361

6462
// filesettings
6563
// check all files of the project
6664
for (const FileSettings &fs : mFileSettings) {
67-
if (!mSettings.library.markupFile(fs.filename) || !mSettings.library.processMarkupAfterCode(fs.filename)) {
68-
result += mCppcheck.check(fs);
69-
++c;
70-
if (!mSettings.quiet)
71-
reportStatus(c, mFileSettings.size(), c, mFileSettings.size());
72-
if (mSettings.clangTidy)
73-
mCppcheck.analyseClangTidy(fs);
74-
}
75-
}
76-
77-
// second loop to parse all markup files which may not work until all
78-
// c/cpp files have been parsed and checked
79-
// TODO: get rid of duplicated code
80-
for (std::list<std::pair<std::string, std::size_t>>::const_iterator i = mFiles.cbegin(); i != mFiles.cend(); ++i) {
81-
if (mSettings.library.markupFile(i->first) && mSettings.library.processMarkupAfterCode(i->first)) {
82-
result += mCppcheck.check(i->first);
83-
processedsize += i->second;
84-
++c;
85-
if (!mSettings.quiet)
86-
reportStatus(c, mFiles.size(), processedsize, totalfilesize);
87-
// TODO: call analyseClangTidy()?
88-
}
89-
}
90-
91-
for (const FileSettings &fs : mFileSettings) {
92-
if (mSettings.library.markupFile(fs.filename) && mSettings.library.processMarkupAfterCode(fs.filename)) {
93-
result += mCppcheck.check(fs);
94-
++c;
95-
if (!mSettings.quiet)
96-
reportStatus(c, mFileSettings.size(), c, mFileSettings.size());
97-
if (mSettings.clangTidy)
98-
mCppcheck.analyseClangTidy(fs);
99-
}
65+
result += mCppcheck.check(fs);
66+
++c;
67+
if (!mSettings.quiet)
68+
reportStatus(c, mFileSettings.size(), c, mFileSettings.size());
69+
if (mSettings.clangTidy)
70+
mCppcheck.analyseClangTidy(fs);
10071
}
10172

10273
if (mCppcheck.analyseWholeProgram())

‎test/cli/test-other.py

+60
Original file line numberDiff line numberDiff line change
@@ -664,3 +664,63 @@ def test_file_order(tmpdir):
664664
'4/4 files checked 0% done'
665665
]
666666
assert stderr == ''
667+
668+
669+
def test_markup(tmpdir):
670+
test_file_1 = os.path.join(tmpdir, 'test_1.qml')
671+
with open(test_file_1, 'wt') as f:
672+
pass
673+
test_file_2 = os.path.join(tmpdir, 'test_2.cpp')
674+
with open(test_file_2, 'wt') as f:
675+
pass
676+
test_file_3 = os.path.join(tmpdir, 'test_3.qml')
677+
with open(test_file_3, 'wt') as f:
678+
pass
679+
test_file_4 = os.path.join(tmpdir, 'test_4.cpp')
680+
with open(test_file_4, 'wt') as f:
681+
pass
682+
683+
args = ['--library=qt', test_file_1, test_file_2, test_file_3, test_file_4]
684+
out_lines = [
685+
'Checking {} ...'.format(test_file_2),
686+
'1/4 files checked 0% done',
687+
'Checking {} ...'.format(test_file_4),
688+
'2/4 files checked 0% done',
689+
'Checking {} ...'.format(test_file_1),
690+
'3/4 files checked 0% done',
691+
'Checking {} ...'.format(test_file_3),
692+
'4/4 files checked 0% done'
693+
]
694+
695+
assert_cppcheck(args, ec_exp=0, err_exp=[], out_exp=out_lines)
696+
697+
698+
def test_markup_j(tmpdir):
699+
test_file_1 = os.path.join(tmpdir, 'test_1.qml')
700+
with open(test_file_1, 'wt') as f:
701+
pass
702+
test_file_2 = os.path.join(tmpdir, 'test_2.cpp')
703+
with open(test_file_2, 'wt') as f:
704+
pass
705+
test_file_3 = os.path.join(tmpdir, 'test_3.qml')
706+
with open(test_file_3, 'wt') as f:
707+
pass
708+
test_file_4 = os.path.join(tmpdir, 'test_4.cpp')
709+
with open(test_file_4, 'wt') as f:
710+
pass
711+
712+
args = ['--library=qt', test_file_1, test_file_2, test_file_3, test_file_4]
713+
714+
exitcode, stdout, stderr = cppcheck(args)
715+
assert exitcode == 0
716+
lines = stdout.splitlines()
717+
for i in range(1, 5):
718+
lines.remove('{}/4 files checked 0% done'.format(i))
719+
720+
assert lines == [
721+
'Checking {} ...'.format(test_file_2),
722+
'Checking {} ...'.format(test_file_4),
723+
'Checking {} ...'.format(test_file_1),
724+
'Checking {} ...'.format(test_file_3)
725+
]
726+
assert stderr == ''

‎test/cli/test-qml.py

+14
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,27 @@
22
# python3 -m pytest test-qml.py
33

44
import os
5+
import pytest
56
from testutils import cppcheck
67

78
PROJECT_DIR = 'QML-Samples-TableView'
89

10+
911
def test_unused_functions():
1012
ret, stdout, stderr = cppcheck(['--library=qt', '--enable=unusedFunction', PROJECT_DIR])
1113
# there are unused functions. But fillSampleData is not unused because that is referenced from main.qml
1214
assert '[unusedFunction]' in stderr
1315
assert 'fillSampleData' not in stderr
1416

17+
18+
@pytest.mark.xfail
19+
def test_unused_functions_j(tmpdir):
20+
build_dir = os.path.join(tmpdir, 'b1')
21+
os.mkdir(build_dir)
22+
ret, stdout, stderr = cppcheck(['--library=qt', '--enable=unusedFunction', '-j2', '--cppcheck-build-dir={}'.format(build_dir), PROJECT_DIR])
23+
# there are unused functions. But fillSampleData is not unused because that is referenced from main.qml
24+
assert '[unusedFunction]' in stderr
25+
assert 'fillSampleData' not in stderr
26+
27+
# TODO: test with project file
28+
# TODO: test with FileSettings

‎test/cli/testutils.py

+2-3
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
21
import logging
32
import os
43
import subprocess
@@ -83,7 +82,7 @@ def assert_cppcheck(args, ec_exp=None, out_exp=None, err_exp=None, env=None):
8382
assert exitcode == ec_exp, stdout
8483
if out_exp is not None:
8584
out_lines = stdout.splitlines()
86-
assert out_lines == out_exp
85+
assert out_lines == out_exp, stdout
8786
if err_exp is not None:
8887
err_lines = stderr.splitlines()
89-
assert err_lines == err_exp
88+
assert err_lines == err_exp, stderr

‎test/testprocessexecutor.cpp

-41
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,6 @@ class TestProcessExecutorBase : public TestFixture {
147147
TEST_CASE(no_errors_equal_amount_files);
148148
TEST_CASE(one_error_less_files);
149149
TEST_CASE(one_error_several_files);
150-
TEST_CASE(markup);
151150
TEST_CASE(clangTidy);
152151
TEST_CASE(showtime_top5_file);
153152
TEST_CASE(showtime_top5_summary);
@@ -244,46 +243,6 @@ class TestProcessExecutorBase : public TestFixture {
244243
"}");
245244
}
246245

247-
void markup() {
248-
const Settings settingsOld = settings;
249-
settings.library.mMarkupExtensions.emplace(".cp1");
250-
settings.library.mProcessAfterCode.emplace(".cp1", true);
251-
252-
const std::vector<std::string> files = {
253-
fprefix() + "_1.cp1", fprefix() + "_2.cpp", fprefix() + "_3.cp1", fprefix() + "_4.cpp"
254-
};
255-
256-
// the checks are not executed on the markup files => expected result is 2
257-
check(2, 4, 2,
258-
"int main()\n"
259-
"{\n"
260-
" int i = *((int*)0);\n"
261-
" return 0;\n"
262-
"}",
263-
dinit(CheckOptions,
264-
$.quiet = false,
265-
$.filesList = files));
266-
// TODO: order of "Checking" and "checked" is affected by thread
267-
/*TODO_ASSERT_EQUALS("Checking " + fprefix() + "_2.cpp ...\n"
268-
"1/4 files checked 25% done\n"
269-
"Checking " + fprefix() + "_4.cpp ...\n"
270-
"2/4 files checked 50% done\n"
271-
"Checking " + fprefix() + "_1.cp1 ...\n"
272-
"3/4 files checked 75% done\n"
273-
"Checking " + fprefix() + "_3.cp1 ...\n"
274-
"4/4 files checked 100% done\n",
275-
"Checking " + fprefix() + "_1.cp1 ...\n"
276-
"1/4 files checked 25% done\n"
277-
"Checking " + fprefix() + "_2.cpp ...\n"
278-
"2/4 files checked 50% done\n"
279-
"Checking " + fprefix() + "_3.cp1 ...\n"
280-
"3/4 files checked 75% done\n"
281-
"Checking " + fprefix() + "_4.cpp ...\n"
282-
"4/4 files checked 100% done\n",
283-
output.str());*/
284-
settings = settingsOld;
285-
}
286-
287246
void clangTidy() {
288247
// TODO: we currently only invoke it with ImportProject::FileSettings
289248
if (!useFS)

‎test/testsingleexecutor.cpp

-32
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,6 @@ class TestSingleExecutorBase : public TestFixture {
151151
TEST_CASE(no_errors_equal_amount_files);
152152
TEST_CASE(one_error_less_files);
153153
TEST_CASE(one_error_several_files);
154-
TEST_CASE(markup);
155154
TEST_CASE(clangTidy);
156155
TEST_CASE(showtime_top5_file);
157156
TEST_CASE(showtime_top5_summary);
@@ -240,37 +239,6 @@ class TestSingleExecutorBase : public TestFixture {
240239
"}");
241240
}
242241

243-
void markup() {
244-
const Settings settingsOld = settings;
245-
settings.library.mMarkupExtensions.emplace(".cp1");
246-
settings.library.mProcessAfterCode.emplace(".cp1", true);
247-
248-
const std::vector<std::string> files = {
249-
fprefix() + "_1.cp1", fprefix() + "_2.cpp", fprefix() + "_3.cp1", fprefix() + "_4.cpp"
250-
};
251-
252-
// checks are not executed on markup files => expected result is 2
253-
check(4, 2,
254-
"int main()\n"
255-
"{\n"
256-
" int i = *((int*)0);\n"
257-
" return 0;\n"
258-
"}",
259-
dinit(CheckOptions,
260-
$.quiet = false,
261-
$.filesList = files));
262-
// TODO: filter out the "files checked" messages
263-
ASSERT_EQUALS("Checking " + fprefix() + "_2.cpp ...\n"
264-
"1/4 files checked 25% done\n"
265-
"Checking " + fprefix() + "_4.cpp ...\n"
266-
"2/4 files checked 50% done\n"
267-
"Checking " + fprefix() + "_1.cp1 ...\n"
268-
"3/4 files checked 75% done\n"
269-
"Checking " + fprefix() + "_3.cp1 ...\n"
270-
"4/4 files checked 100% done\n", output.str());
271-
settings = settingsOld;
272-
}
273-
274242
void clangTidy() {
275243
// TODO: we currently only invoke it with ImportProject::FileSettings
276244
if (!useFS)

0 commit comments

Comments
 (0)