Skip to content

Commit 27ce21e

Browse files
firewavefirewave
committed
do not unconditionally apply colors to output / disable all colors in tests / adjusted tests for changed output behavior
1 parent b71f604 commit 27ce21e

10 files changed

Lines changed: 81 additions & 41 deletions

‎cli/cppcheckexecutor.cpp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -379,8 +379,10 @@ void CppCheckExecutor::reportErr(const std::string &errmsg)
379379

380380
void CppCheckExecutor::reportOut(const std::string &outmsg, Color c)
381381
{
382-
// TODO: do not unconditionally apply colors
383-
std::cout << c << ansiToOEM(outmsg, true) << Color::Reset << std::endl;
382+
if (c == Color::Reset)
383+
std::cout << ansiToOEM(outmsg, true) << std::endl;
384+
else
385+
std::cout << toString(c) << ansiToOEM(outmsg, true) << toString(Color::Reset) << std::endl;
384386
}
385387

386388
void CppCheckExecutor::reportProgress(const std::string &filename, const char stage[], const std::size_t value)

‎cli/executor.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,7 @@ void Executor::reportStatus(std::size_t fileindex, std::size_t filecount, std::s
5858
oss << fileindex << '/' << filecount
5959
<< " files checked " << percentDone
6060
<< "% done";
61-
// TODO: do not unconditionally print in color
62-
std::cout << Color::FgBlue << oss.str() << Color::Reset << std::endl;
61+
mErrorLogger.reportOut(oss.str(), Color::FgBlue);
6362
}
6463
}
6564

‎cli/executor.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ class Executor {
5454
* @param sizedone The sum of sizes of the files checked.
5555
* @param sizetotal The total sizes of the files.
5656
*/
57-
static void reportStatus(std::size_t fileindex, std::size_t filecount, std::size_t sizedone, std::size_t sizetotal);
57+
void reportStatus(std::size_t fileindex, std::size_t filecount, std::size_t sizedone, std::size_t sizetotal);
5858

5959
protected:
6060
/**

‎cli/threadexecutor.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ class SyncLogForwarder : public ErrorLogger
6868

6969
void reportStatus(std::size_t fileindex, std::size_t filecount, std::size_t sizedone, std::size_t sizetotal) {
7070
std::lock_guard<std::mutex> lg(mReportSync);
71-
Executor::reportStatus(fileindex, filecount, sizedone, sizetotal);
71+
mThreadExecutor.reportStatus(fileindex, filecount, sizedone, sizetotal);
7272
}
7373

7474
private:

‎lib/color.cpp‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,17 @@
2424
#include <cstddef>
2525
#include <sstream> // IWYU pragma: keep
2626

27+
bool DISABLE_COLORS = false;
28+
2729
#ifdef _WIN32
2830
std::ostream& operator<<(std::ostream& os, const Color& /*c*/)
2931
{
3032
#else
3133
std::ostream& operator<<(std::ostream & os, const Color& c)
3234
{
33-
static const bool use_color = isatty(STDOUT_FILENO);
34-
if (use_color)
35+
// TODO: handle piping into file as well as other pipes like stderr
36+
static const bool s_is_tty = isatty(STDOUT_FILENO);
37+
if (!DISABLE_COLORS && s_is_tty)
3538
return os << "\033[" << static_cast<std::size_t>(c) << "m";
3639
#endif
3740
return os;

‎lib/color.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,4 +42,6 @@ CPPCHECKLIB std::ostream& operator<<(std::ostream& os, const Color& c);
4242

4343
std::string toString(const Color& c);
4444

45+
extern bool DISABLE_COLORS; // for testing
46+
4547
#endif

‎test/main.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
* along with this program. If not, see <http://www.gnu.org/licenses/>.
1717
*/
1818

19+
#include "color.h"
1920
#include "options.h"
2021
#include "preprocessor.h"
2122
#include "fixture.h"
@@ -30,6 +31,7 @@ int main(int argc, char *argv[])
3031
#endif
3132

3233
Preprocessor::macroChar = '$'; // While macroChar is char(1) per default outside test suite, we require it to be a human-readable character here.
34+
DISABLE_COLORS = true;
3335

3436
options args(argc, argv);
3537

‎test/testprocessexecutor.cpp‎

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,6 @@ class TestProcessExecutor : public TestFixture {
131131
const char plistOutput[] = "plist";
132132
ScopedFile plistFile("dummy", plistOutput);
133133

134-
SUPPRESS;
135134
check(16, 100, 100,
136135
"int main()\n"
137136
"{\n"
@@ -185,30 +184,39 @@ class TestProcessExecutor : public TestFixture {
185184

186185
void markup() {
187186
const Settings settingsOld = settings;
187+
settings.quiet = true;
188188
settings.library.mMarkupExtensions.emplace(".cp1");
189189
settings.library.mProcessAfterCode.emplace(".cp1", true);
190190

191191
const std::vector<std::string> files = {
192192
"file_1.cp1", "file_2.cpp", "file_3.cp1", "file_4.cpp"
193193
};
194194

195-
SUPPRESS;
196195
check(2, 4, 4,
197196
"int main()\n"
198197
"{\n"
199198
" char *a = malloc(10);\n"
200199
" return 0;\n"
201200
"}",
202201
SHOWTIME_MODES::SHOWTIME_NONE, nullptr, files);
203-
TODO_ASSERT_EQUALS("\x1b[32mChecking file_2.cpp ...\x1b[0m\n"
204-
"\x1b[32mChecking file_4.cpp ...\x1b[0m\n"
205-
"\x1b[32mChecking file_1.cp1 ...\x1b[0m\n"
206-
"\x1b[32mChecking file_3.cp1 ...\x1b[0m\n",
207-
"\x1b[32mChecking file_1.cp1 ...\x1b[0m\n"
208-
"\x1b[32mChecking file_2.cpp ...\x1b[0m\n"
209-
"\x1b[32mChecking file_3.cp1 ...\x1b[0m\n"
210-
"\x1b[32mChecking file_4.cpp ...\x1b[0m\n",
211-
output.str());
202+
// TODO: order of "Checking" and "checked" is affected by thread
203+
/*TODO_ASSERT_EQUALS("Checking file_2.cpp ...\n"
204+
"1/4 files checked 25% done\n"
205+
"Checking file_4.cpp ...\n"
206+
"2/4 files checked 50% done\n"
207+
"Checking file_1.cp1 ...\n"
208+
"3/4 files checked 75% done\n"
209+
"Checking file_3.cp1 ...\n"
210+
"4/4 files checked 100% done\n",
211+
"Checking file_1.cp1 ...\n"
212+
"1/4 files checked 25% done\n"
213+
"Checking file_2.cpp ...\n"
214+
"2/4 files checked 50% done\n"
215+
"Checking file_3.cp1 ...\n"
216+
"3/4 files checked 75% done\n"
217+
"Checking file_4.cpp ...\n"
218+
"4/4 files checked 100% done\n",
219+
output.str());*/
212220
}
213221
};
214222

‎test/testsingleexecutor.cpp‎

Lines changed: 33 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
* This program is distributed in the hope that it will be useful,
1111
* but WITHOUT ANY WARRANTY; without even the implied warranty of
1212
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13-
* GNU General Public License for more details.
13+
* GNU General Public License for more details
1414
*
1515
* You should have received a copy of the GNU General Public License
1616
* along with this program. If not, see <http://www.gnu.org/licenses/>.
@@ -43,16 +43,24 @@ class TestSingleExecutor : public TestFixture {
4343
private:
4444
Settings settings;
4545

46+
static std::string zpad3(int i)
47+
{
48+
if (i < 10)
49+
return "00" + std::to_string(i);
50+
if (i < 100)
51+
return "0" + std::to_string(i);
52+
return std::to_string(i);
53+
}
54+
4655
void check(int files, int result, const std::string &data, SHOWTIME_MODES showtime = SHOWTIME_MODES::SHOWTIME_NONE, const char* const plistOutput = nullptr, const std::vector<std::string>& filesList = {}) {
4756
errout.str("");
4857
output.str("");
4958

5059
std::map<std::string, std::size_t> filemap;
5160
if (filesList.empty()) {
5261
for (int i = 1; i <= files; ++i) {
53-
std::ostringstream oss;
54-
oss << "file_" << i << ".cpp";
55-
filemap[oss.str()] = data.size();
62+
const std::string s = "file_" + zpad3(i) + ".cpp";
63+
filemap[s] = data.size();
5664
}
5765
}
5866
else {
@@ -95,7 +103,9 @@ class TestSingleExecutor : public TestFixture {
95103
}
96104

97105
void many_files() {
98-
REDIRECT;
106+
const Settings settingsOld = settings;
107+
settings.quiet = false;
108+
99109
check(100, 100,
100110
"int main()\n"
101111
"{\n"
@@ -104,6 +114,7 @@ class TestSingleExecutor : public TestFixture {
104114
"}");
105115
std::string expected;
106116
for (int i = 1; i <= 100; ++i) {
117+
expected += "Checking file_" + zpad3(i) + ".cpp ...\n";
107118
int p;
108119
if (i == 29)
109120
p = 28;
@@ -113,26 +124,27 @@ class TestSingleExecutor : public TestFixture {
113124
p = 57;
114125
else
115126
p = i;
116-
expected += "\x1b[34m" + std::to_string(i) + "/100 files checked " + std::to_string(p) + "% done\x1b[0m\n";
127+
expected += std::to_string(i) + "/100 files checked " + std::to_string(p) + "% done\n";
117128
}
118-
ASSERT_EQUALS(expected, GET_REDIRECT_OUTPUT);
129+
ASSERT_EQUALS(expected, output.str());
130+
131+
settings = settingsOld;
119132
}
120133

121134
void many_files_showtime() {
122135
SUPPRESS;
123-
check( 100, 100,
124-
"int main()\n"
125-
"{\n"
126-
" char *a = malloc(10);\n"
127-
" return 0;\n"
128-
"}", SHOWTIME_MODES::SHOWTIME_SUMMARY);
136+
check(100, 100,
137+
"int main()\n"
138+
"{\n"
139+
" char *a = malloc(10);\n"
140+
" return 0;\n"
141+
"}", SHOWTIME_MODES::SHOWTIME_SUMMARY);
129142
}
130143

131144
void many_files_plist() {
132145
const char plistOutput[] = "plist";
133146
ScopedFile plistFile("dummy", plistOutput);
134147

135-
SUPPRESS;
136148
check(100, 100,
137149
"int main()\n"
138150
"{\n"
@@ -187,23 +199,28 @@ class TestSingleExecutor : public TestFixture {
187199
const Settings settingsOld = settings;
188200
settings.library.mMarkupExtensions.emplace(".cp1");
189201
settings.library.mProcessAfterCode.emplace(".cp1", true);
202+
settings.quiet = false;
190203

191204
const std::vector<std::string> files = {
192205
"file_1.cp1", "file_2.cpp", "file_3.cp1", "file_4.cpp"
193206
};
194207

195-
SUPPRESS;
196208
check(4, 4,
197209
"int main()\n"
198210
"{\n"
199211
" char *a = malloc(10);\n"
200212
" return 0;\n"
201213
"}",
202214
SHOWTIME_MODES::SHOWTIME_NONE, nullptr, files);
215+
// TODO: filter out the "files checked" messages
203216
ASSERT_EQUALS("Checking file_2.cpp ...\n"
217+
"1/4 files checked 25% done\n"
204218
"Checking file_4.cpp ...\n"
219+
"2/4 files checked 50% done\n"
205220
"Checking file_1.cp1 ...\n"
206-
"Checking file_3.cp1 ...\n", output.str());
221+
"3/4 files checked 75% done\n"
222+
"Checking file_3.cp1 ...\n"
223+
"4/4 files checked 100% done\n", output.str());
207224
}
208225

209226
// TODO: test clang-tidy

‎test/testthreadexecutor.cpp‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,6 @@ class TestThreadExecutor : public TestFixture {
129129
const char plistOutput[] = "plist";
130130
ScopedFile plistFile("dummy", plistOutput);
131131

132-
SUPPRESS;
133132
check(16, 100, 100,
134133
"int main()\n"
135134
"{\n"
@@ -189,23 +188,31 @@ class TestThreadExecutor : public TestFixture {
189188
"file_1.cp1", "file_2.cpp", "file_3.cp1", "file_4.cpp"
190189
};
191190

192-
SUPPRESS;
193191
check(2, 4, 4,
194192
"int main()\n"
195193
"{\n"
196194
" char *a = malloc(10);\n"
197195
" return 0;\n"
198196
"}",
199197
SHOWTIME_MODES::SHOWTIME_NONE, nullptr, files);
200-
TODO_ASSERT_EQUALS("Checking file_2.cpp ...\n"
198+
// TODO: order of "Checking" and "checked" is affected by thread
199+
/*TODO_ASSERT_EQUALS("Checking file_2.cpp ...\n"
200+
"1/4 files checked 25% done\n"
201201
"Checking file_4.cpp ...\n"
202+
"2/4 files checked 50% done\n"
202203
"Checking file_1.cp1 ...\n"
203-
"Checking file_3.cp1 ...\n",
204+
"3/4 files checked 75% done\n"
205+
"Checking file_3.cp1 ...\n"
206+
"4/4 files checked 100% done\n",
204207
"Checking file_1.cp1 ...\n"
208+
"1/4 files checked 25% done\n"
205209
"Checking file_2.cpp ...\n"
210+
"2/4 files checked 50% done\n"
206211
"Checking file_3.cp1 ...\n"
207-
"Checking file_4.cpp ...\n",
208-
output.str());
212+
"3/4 files checked 75% done\n"
213+
"Checking file_4.cpp ...\n"
214+
"4/4 files checked 100% done\n",
215+
output.str());*/
209216
}
210217
};
211218

0 commit comments

Comments
 (0)