refs #14599 - added OneShotTimer / moved some ShowTime logic out of Timer

test/cli/test_other.py: added showtime tests with `FileSettings`

Author: firewave

cli/cppcheckexecutor.cpp

@@ -267,8 +267,9 @@ int CppCheckExecutor::check(int argc, const char* const argv[])
         return EXIT_SUCCESS;
     }
 
-    TimerResults overallTimerResults;
-    Timer realTimeClock("Overall time", settings.showtime, &overallTimerResults, Timer::Type::OVERALL);
+    std::unique_ptr<OneShotTimer> overallTimer;
+    if (settings.showtime == ShowTime::SUMMARY || settings.showtime == ShowTime::TOP5_SUMMARY)
+        overallTimer.reset(new OneShotTimer("Overall time", settings.showtime));
 
     settings.loadSummaries();
 
@@ -277,9 +278,6 @@ int CppCheckExecutor::check(int argc, const char* const argv[])
 
     const int ret = check_wrapper(settings, supprs);
 
-    realTimeClock.stop();
-    overallTimerResults.showResults(settings.showtime, false, true);
-
     return ret;
 }
 

lib/cppcheck.cpp

@@ -923,8 +923,9 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
     if (Settings::terminated())
         return mLogger->exitcode();
 
-    TimerResults checkTimeResults;
-    Timer fileTotalTimer{"Check time: " + file.spath(), mSettings.showtime, &checkTimeResults, Timer::Type::FILE};
+    std::unique_ptr<OneShotTimer> checkTimeTimer;
+    if (mSettings.showtime == ShowTime::FILE || mSettings.showtime == ShowTime::FILE_TOTAL || mSettings.showtime == ShowTime::TOP5_FILE)
+        checkTimeTimer.reset(new OneShotTimer("Check time: " + file.spath(), mSettings.showtime));
 
     if (!mSettings.quiet) {
         std::string fixedpath = Path::toNativeSeparators(file.spath());
@@ -1296,9 +1297,6 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
     if (mTimerResults && (mSettings.showtime == ShowTime::FILE || mSettings.showtime == ShowTime::TOP5_FILE))
         mTimerResults->showResults(mSettings.showtime);
 
-    fileTotalTimer.stop();
-    checkTimeResults.showResults(mSettings.showtime, false, true);
-
     return mLogger->exitcode();
 }
 

lib/timer.cpp

@@ -38,7 +38,7 @@ namespace {
 
 // TODO: this does not include any file context when SHOWTIME_FILE thus rendering it useless - should we include the logging with the progress logging?
 // that could also get rid of the broader locking
-void TimerResults::showResults(ShowTime mode, bool metrics, bool format) const
+void TimerResults::showResults(ShowTime mode, bool metrics) const
 {
     if (mode == ShowTime::NONE)
         return;
@@ -60,11 +60,7 @@ void TimerResults::showResults(ShowTime mode, bool metrics, bool format) const
         const double sec = iter->second.getSeconds().count();
         const double secAverage = sec / static_cast<double>(iter->second.mNumberOfResults);
         if ((mode != ShowTime::TOP5_FILE && mode != ShowTime::TOP5_SUMMARY) || (ordinal<=5)) {
-            std::cout << iter->first << ": ";
-            if (format)
-                std::cout << TimerResultsData::durationToString(iter->second.mDuration);
-            else
-                std::cout << sec << "s";
+            std::cout << iter->first << ": " << sec << "s";
             if (metrics)
                 std::cout << " (avg. " << secAverage << "s - " << iter->second.mNumberOfResults  << " result(s))";
             std::cout << std::endl;
@@ -87,10 +83,9 @@ void TimerResults::reset()
     mResults.clear();
 }
 
-Timer::Timer(std::string str, ShowTime showtimeMode, TimerResultsIntf* timerResults, Type type)
+Timer::Timer(std::string str, ShowTime showtimeMode, TimerResultsIntf* timerResults)
     : mName(std::move(str))
     , mMode(showtimeMode)
-    , mType(type)
     , mStart(Clock::now())
     , mResults(timerResults)
 {}
@@ -104,14 +99,6 @@ void Timer::stop()
 {
     if (mMode == ShowTime::NONE)
         return;
-    if (mType == Type::OVERALL && mMode != ShowTime::TOP5_SUMMARY && mMode != ShowTime::SUMMARY) {
-        mMode = ShowTime::NONE;
-        return;
-    }
-    if (mType == Type::FILE && mMode != ShowTime::TOP5_FILE && mMode != ShowTime::FILE && mMode != ShowTime::FILE_TOTAL) {
-        mMode = ShowTime::NONE;
-        return;
-    }
     if (mStart != TimePoint{}) {
         if (!mResults) {
             assert(false);
@@ -124,7 +111,7 @@ void Timer::stop()
     mMode = ShowTime::NONE; // prevent multiple stops
 }
 
-std::string TimerResultsData::durationToString(std::chrono::milliseconds duration)
+static std::string durationToString(std::chrono::milliseconds duration)
 {
     // Extract hours
     auto hours = std::chrono::duration_cast<std::chrono::hours>(duration);
@@ -148,3 +135,24 @@ std::string TimerResultsData::durationToString(std::chrono::milliseconds duratio
         secondsStr.resize(pos + 4); // keep three decimal
     return (ellapsedTime + secondsStr + "s");
 }
+
+OneShotTimer::OneShotTimer(std::string name, ShowTime showtime)
+{
+    if (showtime == ShowTime::NONE)
+        return;
+
+    class MyResults : public TimerResultsIntf
+    {
+    private:
+        void addResults(const std::string &name, std::chrono::milliseconds duration) override
+        {
+            std::lock_guard<std::mutex> l(stdCoutLock);
+
+            // TODO: do not use std::cout directly
+            std::cout << name << ": " << durationToString(duration) << std::endl;
+        }
+    };
+
+    mResults.reset(new MyResults);
+    mTimer.reset(new Timer(std::move(name), showtime, mResults.get()));
+}

lib/timer.h

@@ -25,6 +25,7 @@
 #include <chrono>
 #include <cstdint>
 #include <map>
+#include <memory>
 #include <mutex>
 #include <string>
 #include <utility>
@@ -52,15 +53,13 @@ struct TimerResultsData {
     std::chrono::duration<double> getSeconds() const {
         return std::chrono::duration_cast<std::chrono::duration<double>>(mDuration);
     }
-
-    static std::string durationToString(std::chrono::milliseconds duration);
 };
 
 class CPPCHECKLIB WARN_UNUSED TimerResults : public TimerResultsIntf {
 public:
     TimerResults() = default;
 
-    void showResults(ShowTime mode, bool metrics = true, bool format = false) const;
+    void showResults(ShowTime mode, bool metrics = true) const;
     void addResults(const std::string& str, std::chrono::milliseconds duration) override;
 
     void reset();
@@ -75,13 +74,7 @@ class CPPCHECKLIB Timer {
     using Clock = std::chrono::high_resolution_clock;
     using TimePoint = std::chrono::time_point<Clock>;
 
-    enum class Type : std::uint8_t {
-        FILE,
-        OVERALL,
-        OTHER
-    };
-
-    Timer(std::string str, ShowTime showtimeMode, TimerResultsIntf* timerResults = nullptr, Type type = Type::OTHER);
+    Timer(std::string str, ShowTime showtimeMode, TimerResultsIntf* timerResults = nullptr);
     ~Timer();
 
     Timer(const Timer&) = delete;
@@ -98,10 +91,18 @@ class CPPCHECKLIB Timer {
 private:
     const std::string mName;
     ShowTime mMode{};
-    Type mType{};
     TimePoint mStart;
     TimerResultsIntf* mResults{};
 };
 
+class CPPCHECKLIB OneShotTimer
+{
+public:
+    OneShotTimer(std::string name, ShowTime showtime);
+private:
+    std::unique_ptr<TimerResultsIntf> mResults;
+    std::unique_ptr<Timer> mTimer;
+};
+
 //---------------------------------------------------------------------------
 #endif // timerH

test/cli/other_test.py

@@ -9,7 +9,7 @@
 import subprocess
 import shutil
 
-from testutils import cppcheck, assert_cppcheck, cppcheck_ex, __lookup_cppcheck_exe
+from testutils import cppcheck, assert_cppcheck, cppcheck_ex, __lookup_cppcheck_exe, create_compile_commands
 from xml.etree import ElementTree
 
 
@@ -956,10 +956,9 @@ def test_unused_function_include(tmpdir):
 
 # TODO: test with clang-tidy
 # TODO: test with --addon
-# TODO: test with FileSettings
 # TODO: test with multiple files
-def __test_showtime(tmp_path, showtime, exp_res, exp_last, extra_args=None):
-    test_file = tmp_path / 'test.cpp'
+def __test_showtime(tmp_path, showtime, exp_res, exp_last, use_compdb, extra_args=None):
+    test_file = tmp_path / 'test.cpp'  # the use of C++ is intentional
     with open(test_file, 'wt') as f:
         f.write(
 """
@@ -972,10 +971,17 @@ def __test_showtime(tmp_path, showtime, exp_res, exp_last, extra_args=None):
     args = [
         f'--showtime={showtime}',
         '--quiet',
-        '--inline-suppr',
-        str(test_file)
+        '--inline-suppr'
     ]
 
+    if use_compdb:
+        compdb_file = tmp_path / 'compile_commands.json'
+        create_compile_commands(compdb_file, [test_file])
+
+        args.append(f'--project={compdb_file}')
+    else:
+        args.append(str(test_file))
+
     if extra_args:
         args += extra_args
 
@@ -994,50 +1000,122 @@ def __test_showtime(tmp_path, showtime, exp_res, exp_last, extra_args=None):
     assert stderr == ''
 
 
+def __test_showtime_top5_file(tmp_path, use_compdb):
+    __test_showtime(tmp_path, 'top5_file', 5, 'Check time: ', use_compdb)
+
+
 def test_showtime_top5_file(tmp_path):
-    __test_showtime(tmp_path, 'top5_file', 5, 'Check time: ')
+    __test_showtime_top5_file(tmp_path, False)
+
+
+def test_showtime_top5_file_compdb(tmp_path):
+    __test_showtime_top5_file(tmp_path, True)
+
+
+# TODO: remove extra args when --executor=process works
+def __test_showtime_top5_summary(tmp_path, use_compdb):
+    __test_showtime(tmp_path, 'top5_summary', 5, 'Overall time: ', use_compdb, ['-j1'])
 
 
-# TODO: remove extra args when --executor=process works works
 def test_showtime_top5_summary(tmp_path):
-    __test_showtime(tmp_path, 'top5_summary', 5, 'Overall time: ', ['-j1'])
+    __test_showtime_top5_summary(tmp_path, False)
+
 
+def test_showtime_top5_summary_compdb(tmp_path):
+    __test_showtime_top5_summary(tmp_path, True)
 
-# TODO: remove when --executor=process works works
+
+# TODO: remove when --executor=process works
 def test_showtime_top5_summary_j_thread(tmp_path):
-    __test_showtime(tmp_path, 'top5_summary', 5, 'Overall time: ', ['-j2', '--executor=thread'])
+    __test_showtime(tmp_path, 'top5_summary', 5, 'Overall time: ', False, ['-j2', '--executor=thread'])
+
+
+# TODO: remove when --executor=process works
+def test_showtime_top5_summary_compdb_j_thread(tmp_path):
+    __test_showtime(tmp_path, 'top5_summary', 5, 'Overall time: ', True, ['-j2', '--executor=thread'])
 
 
 # TODO: remove override when fixed
 @pytest.mark.skipif(sys.platform == 'win32', reason="requires ProcessExecutor")
 @pytest.mark.xfail(strict=True)  # TODO: need to transfer the timer results to parent process - see #4452
 def test_showtime_top5_summary_j_process(tmp_path):
-    __test_showtime(tmp_path, 'top5_summary', 5, 'Overall time: ', ['-j2', '--executor=process'])
+    __test_showtime(tmp_path, 'top5_summary', 5, 'Overall time: ', False, ['-j2', '--executor=process'])
+
+
+# TODO: remove override when fixed
+@pytest.mark.skipif(sys.platform == 'win32', reason="requires ProcessExecutor")
+@pytest.mark.xfail(strict=True)  # TODO: need to transfer the timer results to parent process - see #4452
+def test_showtime_top5_summary_compdb_j_process(tmp_path):
+    __test_showtime(tmp_path, 'top5_summary', 5, 'Overall time: ', True, ['-j2', '--executor=process'])
+
+
+def __test_showtime_file(tmp_path, use_compdb):
+    exp_res = 79
+    # project analysis does not call Preprocessor::getConfig()
+    if use_compdb:
+        exp_res -= 1
+    __test_showtime(tmp_path, 'file', exp_res, 'Check time: ', use_compdb)
 
 
 def test_showtime_file(tmp_path):
-    __test_showtime(tmp_path, 'file', 79, 'Check time: ')
+    __test_showtime_file(tmp_path, False)
+
+
+def test_showtime_file_compdb(tmp_path):
+    __test_showtime_file(tmp_path, True)
+
+
+# TODO: remove extra args when --executor=process works
+def __test_showtime_summary(tmp_path, use_compdb):
+    exp_res = 79
+    # project analysis does not call Preprocessor::getConfig()
+    if use_compdb:
+        exp_res -= 1
+    __test_showtime(tmp_path, 'summary', exp_res, 'Overall time: ', use_compdb, ['-j1'])
 
 
-# TODO: remove extra args when --executor=process works works
 def test_showtime_summary(tmp_path):
-    __test_showtime(tmp_path, 'summary', 79, 'Overall time: ', ['-j1'])
+    __test_showtime_summary(tmp_path, False,)
+
 
+def test_showtime_summary_compdb(tmp_path):
+    __test_showtime_summary(tmp_path, True)
 
-# TODO: remove when --executor=process works works
+
+# TODO: remove when --executor=process works
 def test_showtime_summary_j_thread(tmp_path):
-    __test_showtime(tmp_path, 'summary', 79, 'Overall time: ', ['-j2', '--executor=thread'])
+    __test_showtime(tmp_path, 'summary', 79, 'Overall time: ', False, ['-j2', '--executor=thread'])
+
+
+# TODO: remove when --executor=process works
+def test_showtime_summary_compdb_j_thread(tmp_path):
+    __test_showtime(tmp_path, 'summary', 78, 'Overall time: ', True, ['-j2', '--executor=thread'])
 
 
 # TODO: remove override when fixed
 @pytest.mark.skipif(sys.platform == 'win32', reason="requires ProcessExecutor")
 @pytest.mark.xfail(strict=True)  # TODO: need to transfer the timer results to parent process - see #4452
 def test_showtime_summary_j_process(tmp_path):
-    __test_showtime(tmp_path, 'summary', 79, 'Overall time: ', ['-j2', '--executor=process'])
+    __test_showtime(tmp_path, 'summary', 79, 'Overall time: ', False, ['-j2', '--executor=process'])
+
+
+# TODO: remove override when fixed
+@pytest.mark.skipif(sys.platform == 'win32', reason="requires ProcessExecutor")
+@pytest.mark.xfail(strict=True)  # TODO: need to transfer the timer results to parent process - see #4452
+def test_showtime_summary_compdb_j_process(tmp_path):
+    __test_showtime(tmp_path, 'summary', 78, 'Overall time: ', True, ['-j2', '--executor=process'])
+
+
+def __test_showtime_file_total(tmp_path, use_compdb):
+    __test_showtime(tmp_path, 'file-total', 0, 'Check time: ', use_compdb)
 
 
 def test_showtime_file_total(tmp_path):
-    __test_showtime(tmp_path, 'file-total', 0, 'Check time: ')
+    __test_showtime_file_total(tmp_path, False)
+
+
+def test_showtime_file_total_compdb(tmp_path):
+    __test_showtime_file_total(tmp_path, True)
 
 
 def test_showtime_unique(tmp_path):

test/cli/testutils.py

@@ -5,6 +5,7 @@
 import subprocess
 import time
 import tempfile
+import json
 
 # Create Cppcheck project file
 import sys
@@ -45,6 +46,22 @@ def create_gui_project_file(project_file, root_path=None, import_project=None, p
         f.write(cppcheck_xml)
 
 
+def create_compile_commands(compdb_file, files):
+    compdb = []
+    # TODO: bail out on empty list
+    for f in files:
+        compdb.append(
+            {
+                "file": str(f),
+                "directory": os.path.dirname(f),
+                "command": "gcc -c {}".format(os.path.basename(f))
+            }
+        )
+    compdb_j = json.dumps(compdb)
+    with open(compdb_file, 'wt') as f:
+        f.write(compdb_j)
+
+
 def __lookup_cppcheck_exe():
     # path the script is located in
     script_path = os.path.dirname(os.path.realpath(__file__))