From 740d90b5474cd7a73c2d32f33b19e0b94f527665 Mon Sep 17 00:00:00 2001 From: Richard Date: Tue, 16 Jan 2024 19:54:43 -0700 Subject: [PATCH] Update results files when adding tests Fixes #64 --- Contributing.md | 12 +-- Tools/AddTests/AddTests.cpp | 87 +++++++++++----- Tools/TestCases/ToolResults.cpp | 174 ++++++++++++++++++++++++-------- Tools/TestCases/ToolResults.h | 27 +++-- 4 files changed, 218 insertions(+), 82 deletions(-) diff --git a/Contributing.md b/Contributing.md index ecc0599..e091b33 100644 --- a/Contributing.md +++ b/Contributing.md @@ -100,7 +100,8 @@ which tests were executed against the updated version. of the code being refactored. 1. After all the test cases have been added, use the support tool [add-tests](#executable-add-tests) to adjust the temporary test markers with - appropriate test markers for the refactoring, numbering new test cases for you. + appropriate test markers for the refactoring, numbering new test cases for you and + adding the test cases to all the results files that support this refactoring. 1. Build the test suite. You should get no errors, but warnings about missing diffs for your new test cases. 1. Commit the test cases with git to have a record of the source code @@ -118,8 +119,6 @@ which tests were executed against the updated version. 1. Use the support tool [test-results](#executable-test-results) to validate your changes to the results file for the tool you're testing. 1. Run `ctest --preset default` to validate your changes to the results file for all the tools. - Add the newly created test ids to the results files for any other tools that implement - this refactoring with the results column blank. 1. Use the [tool-summary](#executable-tool-summary) support tool to regenerate the summary reports. 1. Issue a pull request with your changes, which should include: @@ -158,7 +157,8 @@ which tests were executed against the updated version. an expected value with an actual value. 1. After all the test cases have been added, use the support tool [add-tests](#executable-add-tests) to adjust the temporary test markers with - appropriate test markers for the refactoring, numbering new test cases for you. + appropriate test markers for the refactoring, numbering new test cases for you and + adding the test cases to all the results files that support this refactoring. 1. Build the test suite. You should get no errors, but warnings about missing diffs for your new test cases. 1. Commit the test case with git to have a record of the source code @@ -176,8 +176,6 @@ which tests were executed against the updated version. 1. Use the support tool [test-results](#executable-test-results) to validate your changes to the results file for the tool you're testing. 1. Run `ctest --preset default` to validate your changes to the results file for all the tools. - Add the newly created test ids to the results files for any other tools that implement - this refactoring with the results column blank. 1. Use the [tool-summary](#executable-tool-summary) support tool to regenerate the summary reports. 1. Issue a pull request with your changes. @@ -308,7 +306,7 @@ tool are updated. ## Executable add-tests -Example: `add-tests RefactorTest R RefactorTest/NewRename.cpp` +Example: `add-tests RefactorTest R RefactorTest/NewRename.cpp results` The executable add-tests scans the test case directory for existing test cases and replaces temporary test markers in a source file with consecutive markers diff --git a/Tools/AddTests/AddTests.cpp b/Tools/AddTests/AddTests.cpp index 3d515ea..5b11de0 100644 --- a/Tools/AddTests/AddTests.cpp +++ b/Tools/AddTests/AddTests.cpp @@ -1,4 +1,5 @@ #include +#include #include #include @@ -17,6 +18,37 @@ std::string g_testPrefix; int g_numTestCases{}; std::string g_sourceFile; std::vector g_sourceLines; +std::vector g_newLabels; + +bool readTestCases(std::string_view testCaseDirectory) +{ + std::vector errors = testCases::scanTestDirectory(testCaseDirectory); + if (!errors.empty()) + { + std::cerr << "Test cases contain errors:\n"; + for (const std::string &error : errors) + { + std::cerr << error << '\n'; + } + return false; + } + return true; +} + +bool setTestCasePrefix(std::string_view prefix) +{ + const std::vector &tests = testCases::getTests(); + auto it = + std::find_if(tests.begin(), tests.end(), [&](const testCases::Test &test) { return test.prefix == prefix; }); + if (it == tests.end()) + { + std::cerr << "Unknown test prefix '" << prefix << "'\n"; + return false; + } + g_testPrefix = it->prefix; + g_numTestCases = static_cast(testCases::getNumTestCases(it->prefix)); + return true; +} bool readSourceFile(std::string_view sourceFile) { @@ -34,12 +66,12 @@ bool readSourceFile(std::string_view sourceFile) g_sourceLines.emplace_back(std::move(line)); } } - std::filesystem::copy_file(sourceFile, std::string{sourceFile} + ".bak"); + copy_file(sourceFile, std::string{sourceFile} + ".bak", std::filesystem::copy_options::overwrite_existing); g_sourceFile = sourceFile; return true; } -void writeTestMarkers() +void writeSourceFile() { std::string marker{"#GOINK#: "}; int testNum{g_numTestCases}; @@ -51,45 +83,46 @@ void writeTestMarkers() { ++testNum; auto markerEnd = line.find_first_of(' ', goink + marker.length()); - line = line.substr(0, goink) + "#TEST#: " + g_testPrefix + std::to_string(testNum) + line.substr(markerEnd); + g_newLabels.emplace_back(g_testPrefix + std::to_string(testNum)); + line = line.substr(0, goink) + "#TEST#: " + g_newLabels.back() + line.substr(markerEnd); } str << line << '\n'; } } -bool readTestCases(std::string_view testCaseDirectory) +void updateResultsFile(std::filesystem::path file) { - std::vector errors = testCases::scanTestDirectory(testCaseDirectory); - if (!errors.empty()) + testCases::ToolResults results(file); + if (results.addTests(g_testPrefix, g_newLabels)) { - std::cerr << "Test cases contain errors:\n"; - for (const std::string &error : errors) - { - std::cerr << error << '\n'; - } - return false; + results.writeResults(); } - return true; } -bool setTestCasePrefix(std::string_view prefix) +void updateResultsDir(std::filesystem::path dir) { - const std::vector &tests = testCases::getTests(); - auto it = - std::find_if(tests.begin(), tests.end(), [&](const testCases::Test &test) { return test.prefix == prefix; }); - if (it == tests.end()) + const auto endsWith = [](const std::string &text, const std::string &suffix) + { return text.length() > suffix.length() && text.substr(text.length() - suffix.length()) == suffix; }; + const auto isResultsFile = [&](const std::filesystem::path &path) + { return endsWith(path.filename().string(), "Results.md"); }; + + for (auto entry : std::filesystem::directory_iterator(dir)) { - std::cerr << "Unknown test prefix '" << prefix << "'\n"; - return false; + const std::filesystem::path path = entry.path(); + if (is_directory(path)) + { + updateResultsDir(path); + } + else if (isResultsFile(path)) + { + updateResultsFile(path); + } } - g_testPrefix = it->prefix; - g_numTestCases = static_cast(testCases::getNumTestCases(it->prefix)); - return true; } int usage(std::string_view program) { - std::cout << "Usage: " << program << " \n"; + std::cout << "Usage: " << program << " \n"; return 1; } @@ -97,16 +130,16 @@ int main(std::vector args) { try { - if (args.size() < 4) + if (args.size() < 5) { return usage(args[0]); } - if (!readTestCases(args[1]) || !setTestCasePrefix(args[2]) || !readSourceFile(args[3])) { return 1; } - writeTestMarkers(); + writeSourceFile(); + updateResultsDir(args[4]); return 0; } catch (const std::exception &bang) diff --git a/Tools/TestCases/ToolResults.cpp b/Tools/TestCases/ToolResults.cpp index e5f74b9..4458d17 100644 --- a/Tools/TestCases/ToolResults.cpp +++ b/Tools/TestCases/ToolResults.cpp @@ -4,46 +4,70 @@ #include #include +#include +#include #include namespace testCases { -bool isTestCaseResult(const std::string &line) +struct TableRow +{ + TableRow(const std::string &line); + + bool valid{}; + std::string first; + std::string second; +}; + +TableRow::TableRow(const std::string &line) { if (line.empty()) { - return false; - } - const auto separatingBar = line.find_first_of('|', 1); - if (separatingBar == std::string::npos) - { - return false; + return; } const auto beginFirstWord = line.find_first_not_of(' ', line[0] == '|' ? 1 : 0); - if (beginFirstWord == separatingBar) - { - return false; - } + const auto separatingBar = line.find_first_of('|', 1); const auto endFirstWord = line.find_last_not_of(" |", separatingBar); - const std::string firstWord = line.substr(beginFirstWord, endFirstWord - beginFirstWord + 1); - if (firstWord.empty() || firstWord == "Case" || firstWord.find_first_not_of('-') == std::string::npos) + first = line.substr(beginFirstWord, endFirstWord - beginFirstWord + 1); + if (first.empty()) { - return false; + return; } const auto beginSecondWord = line.find_first_not_of(" |", separatingBar + 1); if (beginSecondWord == std::string::npos) { // Test label with no result reported - return true; + valid = true; + return; } auto endSecondWord = line.find_first_of(" |", beginSecondWord); if (endSecondWord == std::string::npos) { endSecondWord = line.length(); } - const std::string secondWord = line.substr(beginSecondWord, endSecondWord - beginSecondWord); - return secondWord != "Result" && secondWord.find_first_not_of('-') != std::string::npos; + second = line.substr(beginSecondWord, endSecondWord - beginSecondWord); + valid = true; + ; +} + +bool isTableHeader(const TableRow &row) +{ + return row.valid + && ((row.first == "Case" && row.second == "Result") + || (row.first.find_first_not_of('-') == std::string::npos + && row.second.find_first_not_of('-') == std::string::npos)); +} + +bool isTableHeader(const std::string &line) +{ + return isTableHeader(TableRow(line)); +} + +bool isTestCaseResult(const std::string &line) +{ + const TableRow row(line); + return row.valid && !isTableHeader(row); } std::string getTestLabel(const std::string &line, const std::string &test) @@ -56,6 +80,13 @@ std::string getTestLabel(const std::string &line, const std::string &test) return {}; } +bool ToolResults::TestResultCollection::isMarkedDeprecated(const std::string &label) const +{ + const auto matchesLabel = [&](const TestResult &result) { return result.line.find(label) != std::string::npos; }; + const auto pos = std::find_if(results.begin(), results.end(), matchesLabel); + return pos != results.end() && pos->deprecated; +} + void ToolResults::scanResultsFile() { if (!is_regular_file(m_path)) @@ -103,10 +134,11 @@ void ToolResults::scanResultsFile() continue; } - m_testReports.push_back(prefix); - const std::string test = prefix; - std::vector &results = m_testResults[test]; - std::vector &labels = m_testResultsLabels[test]; + m_testPrefixes.push_back(prefix); + TestResultCollection results; + results.title = title; + results.prefix = prefix; + bool resultsTableSeen{}; while (file) { getLine(); @@ -114,15 +146,26 @@ void ToolResults::scanResultsFile() { break; } - const std::string label = getTestLabel(line, test); - if (!label.empty()) + if (isTableHeader(line)) + { + results.tableHeader.push_back(line); + resultsTableSeen = true; + continue; + } + if (!resultsTableSeen) { - labels.push_back(label); + results.preamble.push_back(line); + continue; } if (!isTestCaseResult(line)) { continue; } + const std::string label = getTestLabel(line, results.prefix); + if (!label.empty()) + { + results.labels.push_back(label); + } const auto separatingBar = line.find('|', line[0] == '|' ? 1 : 0); bool hasResult = line.find_first_not_of(" |", separatingBar) != std::string::npos; const bool deprecated = line.find("(deprecated)", separatingBar) != std::string::npos; @@ -144,18 +187,22 @@ void ToolResults::scanResultsFile() hasResult = false; } } - results.push_back({line, hasResult, deprecated, passed}); + results.results.push_back({line, hasResult, deprecated, passed}); } + m_testResults.emplace_back(std::move(results)); } } -bool ToolResults::markedDeprecated(const std::string &label) +ToolResults::TestResultCollection &ToolResults::getTestResultsForPrefix(const std::string &prefix) { - const std::string prefix = label.substr(0, label.find_first_of("0123456789")); - const std::vector &results = m_testResults[prefix]; - const auto matchesLabel = [&](const TestResult &result) { return result.line.find(label) != std::string::npos; }; - const auto pos = std::find_if(results.begin(), results.end(), matchesLabel); - return pos != results.end() && pos->deprecated; + auto it = std::find_if(m_testResults.begin(), + m_testResults.end(), + [&](const TestResultCollection &results) { return results.prefix == prefix; }); + if (it == m_testResults.end()) + { + throw std::runtime_error("No test results available for " + prefix); + } + return *it; } std::string getLabel(const TestResult &result) @@ -169,21 +216,22 @@ std::string getLabel(const TestResult &result) void ToolResults::checkResults() { - for (const char *testReport : m_testReports) + for (const char *testPrefix : m_testPrefixes) { - const std::vector &labels = m_testResultsLabels[testReport]; + TestResultCollection &results = getTestResultsForPrefix(testPrefix); + const std::vector &labels = results.labels; auto findLabel = [&](const std::string &label) { return std::find(labels.begin(), labels.end(), label) != labels.end(); }; - for (const std::string &deprecated : getDeprecatedLabels(testReport)) + for (const std::string &deprecated : getDeprecatedLabels(testPrefix)) { if (!findLabel(deprecated)) { m_errors.push_back("error: No test results for deprecated test " + deprecated); } } - for (const std::string &testCase : getTestCaseLabels(testReport)) + for (const std::string &testCase : getTestCaseLabels(testPrefix)) { - if (isDeprecatedLabel(testCase) && !markedDeprecated(testCase)) + if (isDeprecatedLabel(testCase) && !results.isMarkedDeprecated(testCase)) { m_errors.push_back("error: Test results for " + testCase + " not marked deprecated."); } @@ -192,7 +240,7 @@ void ToolResults::checkResults() m_errors.push_back("error: No test results for " + testCase); } } - for (const TestResult &result : m_testResults[testReport]) + for (const TestResult &result : results.results) { const std::string label = getLabel(result); if (!result.hasResult) @@ -211,16 +259,22 @@ void ToolResults::checkResults() std::vector ToolResults::getSummary() const { std::vector toolSummary; - for (std::string test : m_testReports) + for (std::string test : m_testPrefixes) { - const auto &testResults = m_testResults.find(test); + auto getTestResults = [this](const std::string &test) + { + return std::find_if(m_testResults.begin(), + m_testResults.end(), + [&](const TestResultCollection &results) { return results.prefix == test; }); + }; + auto testResults = getTestResults(test); if (testResults == m_testResults.end()) { throw std::runtime_error("No test results available for " + test); } TestSummary summary{}; summary.name = test; - for (const TestResult &result : testResults->second) + for (const TestResult &result : testResults->results) { ++summary.numCases; if (result.deprecated) @@ -246,4 +300,44 @@ std::vector ToolResults::getSummary() const return toolSummary; } +bool ToolResults::addTests(const std::string &prefix, const std::vector &labels) +{ + auto matchesPrefix = [&](const char *testPrefix) { return prefix == testPrefix; }; + if (std::find_if(m_testPrefixes.begin(), m_testPrefixes.end(), matchesPrefix) == m_testPrefixes.end()) + { + return false; + } + TestResultCollection &results = getTestResultsForPrefix(prefix); + std::copy(labels.begin(), labels.end(), std::back_inserter(results.labels)); + std::transform(labels.begin(), + labels.end(), + std::back_inserter(results.results), + [](const std::string &label) { return TestResult{label + " |"}; }); + return true; +} + +void ToolResults::writeResults() +{ + std::ofstream str(m_path); + auto copyLines = [&](const std::vector &preamble) + { std::copy(preamble.begin(), preamble.end(), std::ostream_iterator(str, "\n")); }; + copyLines(m_preamble); + bool insertSeparator{!m_preamble.back().empty()}; + for (const TestResultCollection &results : m_testResults) + { + if (insertSeparator) + { + str << '\n'; + } + str << "## " << results.title << "\n"; + copyLines(results.preamble); + copyLines(results.tableHeader); + for (const TestResult &result : results.results) + { + str << result.line << '\n'; + } + insertSeparator = true; + } +} + } // namespace testCases diff --git a/Tools/TestCases/ToolResults.h b/Tools/TestCases/ToolResults.h index 4d60b5e..3d9974f 100644 --- a/Tools/TestCases/ToolResults.h +++ b/Tools/TestCases/ToolResults.h @@ -17,8 +17,6 @@ struct TestResult bool deprecated; bool passed; }; -using ToolTestResults = std::map>; -using ToolTestResultsLabels = std::map>; inline std::string toolNameFromResultsFile(std::filesystem::path path) { @@ -43,8 +41,8 @@ class ToolResults { public: ToolResults(std::filesystem::path path) : - m_path(std::move(path)), - m_name(std::move(toolNameFromResultsFile(path))) + m_path(path), + m_name(toolNameFromResultsFile(path)) { scanResultsFile(); } @@ -65,12 +63,26 @@ class ToolResults { return m_errors; } - bool markedDeprecated(const std::string &label); void checkResults(); std::vector getSummary() const; + bool addTests(const std::string &prefix, const std::vector &labels); + void writeResults(); private: + struct TestResultCollection + { + std::string title; + std::string prefix; + std::vector preamble; + std::vector tableHeader; + std::vector results; + std::vector labels; + + bool isMarkedDeprecated(const std::string &label) const; + }; + void scanResultsFile(); + TestResultCollection &getTestResultsForPrefix(const std::string &prefix); std::filesystem::path m_path; std::string m_name; @@ -78,9 +90,8 @@ class ToolResults std::vector m_errors; std::vector m_diffs; std::vector m_preamble; - std::vector m_testReports; - ToolTestResults m_testResults; - ToolTestResultsLabels m_testResultsLabels; + std::vector m_testPrefixes; + std::vector m_testResults; }; } // namespace testCases