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.
This commit is contained in:
Oliver Stöneberg 2023-11-09 10:17:30 +01:00 committed by GitHub
parent d24074f7b1
commit b0cde34d1a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 122 additions and 172 deletions

View File

@ -186,30 +186,40 @@ bool CppCheckExecutor::parseFromArgs(Settings &settings, int argc, const char* c
logger.printMessage("Please use --suppress for ignoring results from the header files.");
}
const std::vector<std::string>& pathnames = parser.getPathNames();
const std::list<FileSettings>& fileSettings = parser.getFileSettings();
const std::vector<std::string>& pathnamesRef = parser.getPathNames();
const std::list<FileSettings>& fileSettingsRef = parser.getFileSettings();
// the inputs can only be used exclusively - CmdLineParser should already handle this
assert(!(!pathnames.empty() && !fileSettings.empty()));
assert(!(!pathnamesRef.empty() && !fileSettingsRef.empty()));
if (!fileSettings.empty()) {
if (!fileSettingsRef.empty()) {
std::list<FileSettings> fileSettings;
if (!settings.fileFilters.empty()) {
// filter only for the selected filenames from all project files
std::copy_if(fileSettings.cbegin(), fileSettings.cend(), std::back_inserter(mFileSettings), [&](const FileSettings &fs) {
std::copy_if(fileSettingsRef.cbegin(), fileSettingsRef.cend(), std::back_inserter(fileSettings), [&](const FileSettings &fs) {
return matchglobs(settings.fileFilters, fs.filename);
});
if (mFileSettings.empty()) {
if (fileSettings.empty()) {
logger.printError("could not find any files matching the filter.");
return false;
}
}
else {
mFileSettings = fileSettings;
fileSettings = fileSettingsRef;
}
// sort the markup last
std::copy_if(fileSettings.cbegin(), fileSettings.cend(), std::back_inserter(mFileSettings), [&](const FileSettings &fs) {
return !settings.library.markupFile(fs.filename) || !settings.library.processMarkupAfterCode(fs.filename);
});
std::copy_if(fileSettings.cbegin(), fileSettings.cend(), std::back_inserter(mFileSettings), [&](const FileSettings &fs) {
return settings.library.markupFile(fs.filename) && settings.library.processMarkupAfterCode(fs.filename);
});
}
if (!pathnames.empty()) {
std::list<std::pair<std::string, std::size_t>> files;
if (!pathnamesRef.empty()) {
std::list<std::pair<std::string, std::size_t>> filesResolved;
// TODO: this needs to be inlined into PathMatch as it depends on the underlying filesystem
#if defined(_WIN32)
// For Windows we want case-insensitive path matching
@ -219,26 +229,36 @@ bool CppCheckExecutor::parseFromArgs(Settings &settings, int argc, const char* c
#endif
// Execute recursiveAddFiles() to each given file parameter
const PathMatch matcher(ignored, caseSensitive);
for (const std::string &pathname : pathnames) {
const std::string err = FileLister::recursiveAddFiles(files, Path::toNativeSeparators(pathname), settings.library.markupExtensions(), matcher);
for (const std::string &pathname : pathnamesRef) {
const std::string err = FileLister::recursiveAddFiles(filesResolved, Path::toNativeSeparators(pathname), settings.library.markupExtensions(), matcher);
if (!err.empty()) {
// TODO: bail out?
logger.printMessage(err);
}
}
std::list<std::pair<std::string, std::size_t>> files;
if (!settings.fileFilters.empty()) {
std::copy_if(files.cbegin(), files.cend(), std::inserter(mFiles, mFiles.end()), [&](const decltype(files)::value_type& entry) {
std::copy_if(filesResolved.cbegin(), filesResolved.cend(), std::inserter(files, files.end()), [&](const decltype(filesResolved)::value_type& entry) {
return matchglobs(settings.fileFilters, entry.first);
});
if (mFiles.empty()) {
if (files.empty()) {
logger.printError("could not find any files matching the filter.");
return false;
}
}
else {
mFiles = files;
files = std::move(filesResolved);
}
// sort the markup last
std::copy_if(files.cbegin(), files.cend(), std::inserter(mFiles, mFiles.end()), [&](const decltype(files)::value_type& entry) {
return !settings.library.markupFile(entry.first) || !settings.library.processMarkupAfterCode(entry.first);
});
std::copy_if(files.cbegin(), files.cend(), std::inserter(mFiles, mFiles.end()), [&](const decltype(files)::value_type& entry) {
return settings.library.markupFile(entry.first) && settings.library.processMarkupAfterCode(entry.first);
});
}
if (mFiles.empty() && mFileSettings.empty()) {

View File

@ -51,52 +51,23 @@ unsigned int SingleExecutor::check()
unsigned int c = 0;
for (std::list<std::pair<std::string, std::size_t>>::const_iterator i = mFiles.cbegin(); i != mFiles.cend(); ++i) {
if (!mSettings.library.markupFile(i->first) || !mSettings.library.processMarkupAfterCode(i->first)) {
result += mCppcheck.check(i->first);
processedsize += i->second;
++c;
if (!mSettings.quiet)
reportStatus(c, mFiles.size(), processedsize, totalfilesize);
// TODO: call analyseClangTidy()?
}
result += mCppcheck.check(i->first);
processedsize += i->second;
++c;
if (!mSettings.quiet)
reportStatus(c, mFiles.size(), processedsize, totalfilesize);
// TODO: call analyseClangTidy()?
}
// filesettings
// check all files of the project
for (const FileSettings &fs : mFileSettings) {
if (!mSettings.library.markupFile(fs.filename) || !mSettings.library.processMarkupAfterCode(fs.filename)) {
result += mCppcheck.check(fs);
++c;
if (!mSettings.quiet)
reportStatus(c, mFileSettings.size(), c, mFileSettings.size());
if (mSettings.clangTidy)
mCppcheck.analyseClangTidy(fs);
}
}
// second loop to parse all markup files which may not work until all
// c/cpp files have been parsed and checked
// TODO: get rid of duplicated code
for (std::list<std::pair<std::string, std::size_t>>::const_iterator i = mFiles.cbegin(); i != mFiles.cend(); ++i) {
if (mSettings.library.markupFile(i->first) && mSettings.library.processMarkupAfterCode(i->first)) {
result += mCppcheck.check(i->first);
processedsize += i->second;
++c;
if (!mSettings.quiet)
reportStatus(c, mFiles.size(), processedsize, totalfilesize);
// TODO: call analyseClangTidy()?
}
}
for (const FileSettings &fs : mFileSettings) {
if (mSettings.library.markupFile(fs.filename) && mSettings.library.processMarkupAfterCode(fs.filename)) {
result += mCppcheck.check(fs);
++c;
if (!mSettings.quiet)
reportStatus(c, mFileSettings.size(), c, mFileSettings.size());
if (mSettings.clangTidy)
mCppcheck.analyseClangTidy(fs);
}
result += mCppcheck.check(fs);
++c;
if (!mSettings.quiet)
reportStatus(c, mFileSettings.size(), c, mFileSettings.size());
if (mSettings.clangTidy)
mCppcheck.analyseClangTidy(fs);
}
if (mCppcheck.analyseWholeProgram())

View File

@ -664,3 +664,63 @@ def test_file_order(tmpdir):
'4/4 files checked 0% done'
]
assert stderr == ''
def test_markup(tmpdir):
test_file_1 = os.path.join(tmpdir, 'test_1.qml')
with open(test_file_1, 'wt') as f:
pass
test_file_2 = os.path.join(tmpdir, 'test_2.cpp')
with open(test_file_2, 'wt') as f:
pass
test_file_3 = os.path.join(tmpdir, 'test_3.qml')
with open(test_file_3, 'wt') as f:
pass
test_file_4 = os.path.join(tmpdir, 'test_4.cpp')
with open(test_file_4, 'wt') as f:
pass
args = ['--library=qt', test_file_1, test_file_2, test_file_3, test_file_4]
out_lines = [
'Checking {} ...'.format(test_file_2),
'1/4 files checked 0% done',
'Checking {} ...'.format(test_file_4),
'2/4 files checked 0% done',
'Checking {} ...'.format(test_file_1),
'3/4 files checked 0% done',
'Checking {} ...'.format(test_file_3),
'4/4 files checked 0% done'
]
assert_cppcheck(args, ec_exp=0, err_exp=[], out_exp=out_lines)
def test_markup_j(tmpdir):
test_file_1 = os.path.join(tmpdir, 'test_1.qml')
with open(test_file_1, 'wt') as f:
pass
test_file_2 = os.path.join(tmpdir, 'test_2.cpp')
with open(test_file_2, 'wt') as f:
pass
test_file_3 = os.path.join(tmpdir, 'test_3.qml')
with open(test_file_3, 'wt') as f:
pass
test_file_4 = os.path.join(tmpdir, 'test_4.cpp')
with open(test_file_4, 'wt') as f:
pass
args = ['--library=qt', test_file_1, test_file_2, test_file_3, test_file_4]
exitcode, stdout, stderr = cppcheck(args)
assert exitcode == 0
lines = stdout.splitlines()
for i in range(1, 5):
lines.remove('{}/4 files checked 0% done'.format(i))
assert lines == [
'Checking {} ...'.format(test_file_2),
'Checking {} ...'.format(test_file_4),
'Checking {} ...'.format(test_file_1),
'Checking {} ...'.format(test_file_3)
]
assert stderr == ''

View File

@ -2,13 +2,27 @@
# python3 -m pytest test-qml.py
import os
import pytest
from testutils import cppcheck
PROJECT_DIR = 'QML-Samples-TableView'
def test_unused_functions():
ret, stdout, stderr = cppcheck(['--library=qt', '--enable=unusedFunction', PROJECT_DIR])
# there are unused functions. But fillSampleData is not unused because that is referenced from main.qml
assert '[unusedFunction]' in stderr
assert 'fillSampleData' not in stderr
@pytest.mark.xfail
def test_unused_functions_j(tmpdir):
build_dir = os.path.join(tmpdir, 'b1')
os.mkdir(build_dir)
ret, stdout, stderr = cppcheck(['--library=qt', '--enable=unusedFunction', '-j2', '--cppcheck-build-dir={}'.format(build_dir), PROJECT_DIR])
# there are unused functions. But fillSampleData is not unused because that is referenced from main.qml
assert '[unusedFunction]' in stderr
assert 'fillSampleData' not in stderr
# TODO: test with project file
# TODO: test with FileSettings

View File

@ -1,4 +1,3 @@
import logging
import os
import subprocess
@ -83,7 +82,7 @@ def assert_cppcheck(args, ec_exp=None, out_exp=None, err_exp=None, env=None):
assert exitcode == ec_exp, stdout
if out_exp is not None:
out_lines = stdout.splitlines()
assert out_lines == out_exp
assert out_lines == out_exp, stdout
if err_exp is not None:
err_lines = stderr.splitlines()
assert err_lines == err_exp
assert err_lines == err_exp, stderr

View File

@ -147,7 +147,6 @@ private:
TEST_CASE(no_errors_equal_amount_files);
TEST_CASE(one_error_less_files);
TEST_CASE(one_error_several_files);
TEST_CASE(markup);
TEST_CASE(clangTidy);
TEST_CASE(showtime_top5_file);
TEST_CASE(showtime_top5_summary);
@ -244,46 +243,6 @@ private:
"}");
}
void markup() {
const Settings settingsOld = settings;
settings.library.mMarkupExtensions.emplace(".cp1");
settings.library.mProcessAfterCode.emplace(".cp1", true);
const std::vector<std::string> files = {
fprefix() + "_1.cp1", fprefix() + "_2.cpp", fprefix() + "_3.cp1", fprefix() + "_4.cpp"
};
// the checks are not executed on the markup files => expected result is 2
check(2, 4, 2,
"int main()\n"
"{\n"
" int i = *((int*)0);\n"
" return 0;\n"
"}",
dinit(CheckOptions,
$.quiet = false,
$.filesList = files));
// TODO: order of "Checking" and "checked" is affected by thread
/*TODO_ASSERT_EQUALS("Checking " + fprefix() + "_2.cpp ...\n"
"1/4 files checked 25% done\n"
"Checking " + fprefix() + "_4.cpp ...\n"
"2/4 files checked 50% done\n"
"Checking " + fprefix() + "_1.cp1 ...\n"
"3/4 files checked 75% done\n"
"Checking " + fprefix() + "_3.cp1 ...\n"
"4/4 files checked 100% done\n",
"Checking " + fprefix() + "_1.cp1 ...\n"
"1/4 files checked 25% done\n"
"Checking " + fprefix() + "_2.cpp ...\n"
"2/4 files checked 50% done\n"
"Checking " + fprefix() + "_3.cp1 ...\n"
"3/4 files checked 75% done\n"
"Checking " + fprefix() + "_4.cpp ...\n"
"4/4 files checked 100% done\n",
output.str());*/
settings = settingsOld;
}
void clangTidy() {
// TODO: we currently only invoke it with ImportProject::FileSettings
if (!useFS)

View File

@ -151,7 +151,6 @@ private:
TEST_CASE(no_errors_equal_amount_files);
TEST_CASE(one_error_less_files);
TEST_CASE(one_error_several_files);
TEST_CASE(markup);
TEST_CASE(clangTidy);
TEST_CASE(showtime_top5_file);
TEST_CASE(showtime_top5_summary);
@ -240,37 +239,6 @@ private:
"}");
}
void markup() {
const Settings settingsOld = settings;
settings.library.mMarkupExtensions.emplace(".cp1");
settings.library.mProcessAfterCode.emplace(".cp1", true);
const std::vector<std::string> files = {
fprefix() + "_1.cp1", fprefix() + "_2.cpp", fprefix() + "_3.cp1", fprefix() + "_4.cpp"
};
// checks are not executed on markup files => expected result is 2
check(4, 2,
"int main()\n"
"{\n"
" int i = *((int*)0);\n"
" return 0;\n"
"}",
dinit(CheckOptions,
$.quiet = false,
$.filesList = files));
// TODO: filter out the "files checked" messages
ASSERT_EQUALS("Checking " + fprefix() + "_2.cpp ...\n"
"1/4 files checked 25% done\n"
"Checking " + fprefix() + "_4.cpp ...\n"
"2/4 files checked 50% done\n"
"Checking " + fprefix() + "_1.cp1 ...\n"
"3/4 files checked 75% done\n"
"Checking " + fprefix() + "_3.cp1 ...\n"
"4/4 files checked 100% done\n", output.str());
settings = settingsOld;
}
void clangTidy() {
// TODO: we currently only invoke it with ImportProject::FileSettings
if (!useFS)

View File

@ -147,7 +147,6 @@ private:
TEST_CASE(no_errors_equal_amount_files);
TEST_CASE(one_error_less_files);
TEST_CASE(one_error_several_files);
TEST_CASE(markup);
TEST_CASE(clangTidy);
TEST_CASE(showtime_top5_file);
TEST_CASE(showtime_top5_summary);
@ -243,46 +242,6 @@ private:
"}");
}
void markup() {
const Settings settingsOld = settings;
settings.library.mMarkupExtensions.emplace(".cp1");
settings.library.mProcessAfterCode.emplace(".cp1", true);
const std::vector<std::string> files = {
fprefix() + "_1.cp1", fprefix() + "_2.cpp", fprefix() + "_3.cp1", fprefix() + "_4.cpp"
};
// checks are not executed on markup files => expected result is 2
check(2, 4, 2,
"int main()\n"
"{\n"
" int i = *((int*)0);\n"
" return 0;\n"
"}",
dinit(CheckOptions,
$.quiet = false,
$.filesList = files));
// TODO: order of "Checking" and "checked" is affected by thread
/*TODO_ASSERT_EQUALS("Checking " + fprefix() + "_2.cpp ...\n"
"1/4 files checked 25% done\n"
"Checking " + fprefix() + "_4.cpp ...\n"
"2/4 files checked 50% done\n"
"Checking " + fprefix() + "_1.cp1 ...\n"
"3/4 files checked 75% done\n"
"Checking " + fprefix() + "_3.cp1 ...\n"
"4/4 files checked 100% done\n",
"Checking " + fprefix() + "_1.cp1 ...\n"
"1/4 files checked 25% done\n"
"Checking " + fprefix() + "_2.cpp ...\n"
"2/4 files checked 50% done\n"
"Checking " + fprefix() + "_3.cp1 ...\n"
"3/4 files checked 75% done\n"
"Checking " + fprefix() + "_4.cpp ...\n"
"4/4 files checked 100% done\n",
output.str());*/
settings = settingsOld;
}
void clangTidy() {
// TODO: we currently only invoke it with ImportProject::FileSettings
if (!useFS)