fix code review comments

Author: Lana243

server/proto/testgen.proto

@@ -88,7 +88,6 @@ message ProjectContext {
     string projectPath = 2;
     string testDirPath = 3;
     string buildDirRelativePath = 4;
-    string resultsDirRelativePath = 5;
 }
 
 message SettingsContext {

server/src/KleeRunner.cpp

@@ -11,12 +11,12 @@
 #include "utils/KleeUtils.h"
 #include "utils/LogUtils.h"
 #include "utils/stats/CSVReader.h"
+#include "utils/stats/TestsGenerationStats.h"
 
 #include "loguru.h"
 
 #include <fstream>
 #include <utility>
-#include <utils/stats/TestsGenerationStats.h>
 
 using namespace tests;
 
@@ -26,23 +26,6 @@ namespace {
         fs::remove(kleeDir / "run.istats");
     }
 
-    StatsUtils::KleeStats parseKleeStatsReport(const std::string &kleeStatsReport) {
-        std::stringstream ss(kleeStatsReport);
-        StatsUtils::CSVTable parsedCSV = StatsUtils::readCSV(ss, ',');
-        std::map<std::string, std::chrono::milliseconds> timeStats;
-        std::vector<std::string> keys = {"Time(s)", "TSolver(s)", "TResolve(s)"};
-        std::vector<std::chrono::milliseconds> timeValues;
-        for (const auto &key: keys) {
-            if (!CollectionUtils::containsKey(parsedCSV, key)) {
-                LOG_S(WARNING) << StringUtils::stringFormat("Key %s not found in klee-stats report", key);
-                return {};
-            }
-            std::chrono::milliseconds totalTime((int)(1000 * std::stof(parsedCSV[key].back())));
-            timeValues.emplace_back(totalTime);
-        }
-        return { timeValues[0], timeValues[1], timeValues[2] };
-    }
-
     StatsUtils::KleeStats writeKleeStats(const fs::path &kleeOut) {
         ShellExecTask::ExecutionParameters kleeStatsParams("klee-stats",
                                                            {"--utbot-config", kleeOut.string(),
@@ -53,10 +36,10 @@ namespace {
             LOG_S(ERROR) << out;
             return {};
         } else {
-            LOG_S(DEBUG) << "klee-stats report:";
-            LOG_S(DEBUG) << '\n' << out;
+            LOG_S(DEBUG) << "klee-stats report:" << '\n' << out;
         }
-        return parseKleeStatsReport(out);
+        std::stringstream ss(out);
+        return StatsUtils::KleeStats(ss);
     }
 }
 
@@ -181,50 +164,49 @@ static void processMethod(MethodKtests &ktestChunk,
                 std::vector<ConcretizedObject> kTestObjects(
                     ktestData->objects, ktestData->objects + ktestData->n_objects);
 
-                    std::vector<UTBotKTestObject> objects = CollectionUtils::transform(
-                        kTestObjects, [](const ConcretizedObject &kTestObject) {
-                            return UTBotKTestObject{ kTestObject };
-                        });
+                std::vector<UTBotKTestObject> objects = CollectionUtils::transform(
+                    kTestObjects, [](const ConcretizedObject &kTestObject) {
+                        return UTBotKTestObject{ kTestObject };
+                    });
 
                 std::vector<std::string> errorDescriptors = CollectionUtils::transform(
-                        errorDescriptorFiles, [](const fs::path &errorFile) {
-                            std::ifstream fileWithError(errorFile.c_str(), std::ios_base::in);
-                            std::string content((std::istreambuf_iterator<char>(fileWithError)),
-                                                std::istreambuf_iterator<char>());
-
-                            const std::string &errorId = errorFile.stem().extension().string();
-                            if (!errorId.empty()) {
-                                // skip leading dot
-                                content += "\n" + sarif::ERROR_ID_KEY + ":" + errorId.substr(1);
-                            }
-                            return content;
-                        });
-
+                    errorDescriptorFiles, [](const fs::path &errorFile) {
+                        std::ifstream fileWithError(errorFile.c_str(), std::ios_base::in);
+                        std::string content((std::istreambuf_iterator<char>(fileWithError)),
+                                            std::istreambuf_iterator<char>());
+
+                        const std::string &errorId = errorFile.stem().extension().string();
+                        if (!errorId.empty()) {
+                            // skip leading dot
+                            content += "\n" + sarif::ERROR_ID_KEY + ":" + errorId.substr(1);
+                        }
+                        return content;
+                    });
 
                 ktestChunk[method].emplace_back(objects, status, errorDescriptors);
-                }
             }
         }
-        if (hasTimeout) {
-            std::string message = StringUtils::stringFormat(
-                "Some tests for function '%s' were skipped, as execution of function is "
-                "out of timeout.",
-                method.methodName);
-            tests.commentBlocks.emplace_back(std::move(message));
-        }
-        if (hasError) {
-            std::string message = StringUtils::stringFormat(
-                "Some tests for function '%s' were skipped, as execution of function leads "
-                "KLEE to the internal error. See console log for more details.",
-                method.methodName);
-            tests.commentBlocks.emplace_back(std::move(message));
-        }
+    }
+    if (hasTimeout) {
+        std::string message = StringUtils::stringFormat(
+            "Some tests for function '%s' were skipped, as execution of function is "
+            "out of timeout.",
+            method.methodName);
+        tests.commentBlocks.emplace_back(std::move(message));
+    }
+    if (hasError) {
+        std::string message = StringUtils::stringFormat(
+            "Some tests for function '%s' were skipped, as execution of function leads "
+            "KLEE to the internal error. See console log for more details.",
+            method.methodName);
+        tests.commentBlocks.emplace_back(std::move(message));
+    }
 
-        if (!CollectionUtils::containsKey(ktestChunk, method) || ktestChunk.at(method).empty()) {
-            tests.commentBlocks.emplace_back(StringUtils::stringFormat(
-                "Tests for %s were not generated. Maybe the function is too complex.",
-                method.methodName));
-        }
+    if (!CollectionUtils::containsKey(ktestChunk, method) || ktestChunk.at(method).empty()) {
+        tests.commentBlocks.emplace_back(StringUtils::stringFormat(
+            "Tests for %s were not generated. Maybe the function is too complex.",
+            method.methodName));
+    }
 }
 
 void KleeRunner::processBatchWithoutInteractive(const std::vector<tests::TestMethod> &testMethods,
@@ -237,9 +219,9 @@ void KleeRunner::processBatchWithoutInteractive(const std::vector<tests::TestMet
     for (const auto &testMethod : testMethods) {
         if (testMethod.sourceFilePath != tests.sourceFilePath) {
             std::string message = StringUtils::stringFormat(
-                    "While generating tests for source file: %s tried to generate tests for method %s "
-                    "from another source file: %s. This can cause invalid generation.\n",
-                    tests.sourceFilePath, testMethod.methodName, testMethod.sourceFilePath);
+                "While generating tests for source file: %s tried to generate tests for method %s "
+                "from another source file: %s. This can cause invalid generation.\n",
+                tests.sourceFilePath, testMethod.methodName, testMethod.sourceFilePath);
             LOG_S(WARNING) << message;
         }
 

server/src/KleeRunner.h

@@ -7,11 +7,11 @@
 #include "Tests.h"
 #include "streams/tests/TestsWriter.h"
 #include "utils/stats/KleeStats.h"
+#include "utils/stats/TestsGenerationStats.h"
 
 #include <grpcpp/grpcpp.h>
 
 #include <vector>
-#include <utils/stats/TestsGenerationStats.h>
 
 class KleeRunner {
 public:

server/src/Paths.cpp

@@ -164,17 +164,16 @@ namespace Paths {
     fs::path kleeOutDirForFilePath(const utbot::ProjectContext &projectContext, const fs::path &projectTmpPath,
                                    const fs::path &filePath) {
         fs::path kleeOutDir = getKleeOutDir(projectTmpPath);
-        fs::path relative = (fs::relative(addOrigExtensionAsSuffixAndAddNew(filePath, ""), projectContext.projectPath));
+        fs::path relative = fs::relative(addOrigExtensionAsSuffixAndAddNew(filePath, ""), projectContext.projectPath);
         return kleeOutDir / relative;
     }
 
     fs::path kleeOutDirForEntrypoints(const utbot::ProjectContext &projectContext, const fs::path &projectTmpPath,
                                       const fs::path &srcFilePath, const std::string &methodName) {
-        if (!methodName.empty()) {
-            return kleeOutDirForFilePath(projectContext, projectTmpPath, srcFilePath) / ("klee_out_" + methodName);
-        }
-        return kleeOutDirForFilePath(projectContext, projectTmpPath, srcFilePath) /
-               ("klee_out_" + srcFilePath.filename().stem().string());
+        auto kleeOutDirForFile = kleeOutDirForFilePath(projectContext, projectTmpPath, srcFilePath);
+        std::string suffix = methodName.empty() ?
+                addOrigExtensionAsSuffixAndAddNew(srcFilePath, "").filename().string() : methodName;
+        return kleeOutDirForFile / ("klee_out_" + suffix);
     }
 
     //endregion
@@ -204,7 +203,7 @@ namespace Paths {
     }
 
     fs::path getArtifactsRootDir(const utbot::ProjectContext &projectContext) {
-        return projectContext.buildDir / "utbot";
+        return projectContext.buildDir() / "utbot";
     }
     fs::path getGTestResultsJsonPath(const utbot::ProjectContext &projectContext) {
         return getArtifactsRootDir(projectContext) / "gtest-results.json";
@@ -256,7 +255,7 @@ namespace Paths {
             newFilename = fs::relative(filePath, projectContext.projectPath);
             newFilename = addExtension(newFilename, ".o");
         } else {
-            newFilename = fs::relative(filePath, projectContext.buildDir);
+            newFilename = fs::relative(filePath, projectContext.buildDir());
         }
         return getRecompiledDir(projectContext) / newFilename;
     }
@@ -409,7 +408,7 @@ namespace Paths {
         return removeSuffix(srcPath, STUB_SUFFIX).stem() == headerPath.stem();
     }
     fs::path getUtbotBuildDir(const utbot::ProjectContext &projectContext) {
-        return projectContext.buildDir / CompilationUtils::UTBOT_BUILD_DIR_NAME;
+        return projectContext.buildDir() / CompilationUtils::UTBOT_BUILD_DIR_NAME;
     }
 
     //endregion

server/src/Paths.h

@@ -415,13 +415,19 @@ namespace Paths {
 
     //endregion
 
-    //region stats
+    //region utbot-report
+
+    inline fs::path getUTBotReportDir(const utbot::ProjectContext &projectContext) {
+        return projectContext.projectPath / "utbot-report";
+    }
+
     inline fs::path getGenerationStatsCSVPath(const utbot::ProjectContext &projectContext) {
-        return projectContext.resultsDirPath / "generation-stats.csv";
+        return getUTBotReportDir(projectContext) / "generation-stats.csv";
     }
     inline fs::path getExecutionStatsCSVPath(const utbot::ProjectContext &projectContext) {
-        return projectContext.resultsDirPath / "execution-stats.csv";
+        return getUTBotReportDir(projectContext) / "execution-stats.csv";
     }
+
     //endregion
 
     bool isHeadersEqual(const fs::path &srcPath, const fs::path &headerPath);

server/src/ProjectContext.cpp

@@ -5,38 +5,27 @@
 #include <utility>
 
 namespace utbot {
-    ProjectContext::ProjectContext(std::string projectName,
-                                   fs::path projectPath,
-                                   fs::path testDirPath,
-                                   fs::path buildDirRelativePath,
-                                   fs::path resultsDirRelativePath)
-        : projectName(std::move(projectName)),
-          projectPath(std::move(projectPath)),
-          testDirPath(std::move(testDirPath)),
-          buildDirRelativePath(std::move(buildDirRelativePath)),
-          buildDir(this->projectPath / this->buildDirRelativePath),
-          resultsDirRelativePath(std::move(resultsDirRelativePath)),
-          resultsDirPath(this->projectPath / this->resultsDirRelativePath) { }
-
     ProjectContext::ProjectContext(std::string projectName,
                                    fs::path projectPath,
                                    fs::path testDirPath,
                                    fs::path buildDirRelativePath)
-            : ProjectContext(projectName, projectPath, testDirPath, buildDirRelativePath, "utbot-results") { }
+            : projectName(std::move(projectName)), projectPath(std::move(projectPath)),
+              testDirPath(std::move(testDirPath)),
+              buildDirRelativePath(std::move(buildDirRelativePath)) {}
 
     ProjectContext::ProjectContext(const testsgen::ProjectContext &projectContext)
         : ProjectContext(projectContext.projectname(),
                          projectContext.projectpath(),
                          projectContext.testdirpath(),
-                         projectContext.builddirrelativepath(),
-                         !projectContext.resultsdirrelativepath().empty() ?
-                            projectContext.resultsdirrelativepath() :
-                            "utbot-results") {}
+                         projectContext.builddirrelativepath()) {}
 
     ProjectContext::ProjectContext(const testsgen::SnippetRequest &request, fs::path serverBuildDir)
         : projectName(request.projectcontext().projectname()),
           projectPath(request.projectcontext().projectpath()),
           testDirPath(request.projectcontext().testdirpath()),
-          buildDirRelativePath(request.projectcontext().builddirrelativepath()),
-          buildDir(std::move(serverBuildDir)) { }
+          buildDirRelativePath(request.projectcontext().builddirrelativepath()) { }
+
+    fs::path ProjectContext::buildDir() const {
+        return projectPath / buildDirRelativePath;
+    }
 }

server/src/ProjectContext.h

@@ -18,23 +18,16 @@ class ProjectContext {
                    fs::path testDirPath,
                    fs::path buildDirRelativePath);
 
-    ProjectContext(std::string projectName,
-                   fs::path projectPath,
-                   fs::path testDirPath,
-                   fs::path buildDirRelativePath,
-                   fs::path resultDirRelativePath);
-
     explicit ProjectContext(const testsgen::ProjectContext &projectContext);
 
     ProjectContext(const testsgen::SnippetRequest &request, fs::path serverBuildDir);
 
+    [[nodiscard]] fs::path buildDir() const;
+
     const std::string projectName;
     const fs::path projectPath;
     const fs::path testDirPath;
     const fs::path buildDirRelativePath;
-    const fs::path buildDir;
-    const fs::path resultsDirRelativePath;
-    const fs::path resultsDirPath;
 };
 }
 

server/src/SARIFGenerator.cpp

@@ -76,7 +76,7 @@ namespace sarif {
                                     const fs::path &srcPath = fs::path(stack_match[3]);
                                     const fs::path &relPathInProject = getInProjectPath(projectContext.projectPath, srcPath);
                                     const fs::path &fullPathInProject = projectContext.projectPath / relPathInProject;
-                                    if (Paths::isSubPathOf(projectContext.buildDir, fullPathInProject)) {
+                                    if (Paths::isSubPathOf(projectContext.buildDir(), fullPathInProject)) {
                                         continue;
                                     }
                                     if (!relPathInProject.empty() && fs::exists(fullPathInProject)) {

server/src/Server.cpp

@@ -30,11 +30,11 @@
 #include "utils/LogUtils.h"
 #include "utils/ServerUtils.h"
 #include "utils/stats/TestsGenerationStats.h"
+#include "utils/stats/TestsExecutionStats.h"
 #include "utils/TypeUtils.h"
 
 #include <thread>
 #include <fstream>
-#include <utils/stats/TestsExecutionStats.h>
 
 using TypeUtils::isSameType;
 

server/src/building/BuildDatabase.cpp

@@ -103,7 +103,7 @@ void BuildDatabase::createClangCompileCommandsJson(const fs::path &buildCommands
         objectInfo->command.removeWerror();
         fs::path outputFile = objectInfo->getOutputFile();
         fs::path kleeFilePathTemplate =
-            Paths::createNewDirForFile(sourceFile, projectContext.buildDir, serverBuildDir);
+            Paths::createNewDirForFile(sourceFile, projectContext.buildDir(), serverBuildDir);
         fs::path kleeFile = Paths::addSuffix(kleeFilePathTemplate, "_klee");
         objectInfo->kleeFilesInfo = std::make_shared<KleeFilesInfo>(kleeFile);
 
@@ -676,7 +676,7 @@ std::shared_ptr<BuildDatabase::TargetInfo> BuildDatabase::getPriorityTarget() co
     return *it;
 }
 fs::path BuildDatabase::newDirForFile(const fs::path &file) const {
-    fs::path base = Paths::longestCommonPrefixPath(this->projectContext.buildDir,
+    fs::path base = Paths::longestCommonPrefixPath(this->projectContext.buildDir(),
                                                    this->projectContext.projectPath);
     return Paths::createNewDirForFile(file, base, this->serverBuildDir);
 }