From aa2829e75b693f67bad28d339fed90f202bae365 Mon Sep 17 00:00:00 2001 From: Justin Date: Mon, 16 Sep 2024 18:37:09 -0600 Subject: [PATCH 1/5] Create a private tmp directory for files in the config directory --- include/config/Config.h | 5 ++- include/testharness/TestHarness.h | 15 ++++++++- include/tests/TestFile.h | 29 ++++++++-------- include/tests/TestParser.h | 4 +-- src/config/Config.cpp | 5 +++ src/testharness/TestHarness.cpp | 2 +- src/tests/TestFile.cpp | 25 ++++++-------- src/tests/TestParser.cpp | 56 +++++++++++++++++-------------- src/toolchain/Command.cpp | 6 +++- 9 files changed, 87 insertions(+), 60 deletions(-) diff --git a/include/config/Config.h b/include/config/Config.h index 2979d767..90c32e50 100644 --- a/include/config/Config.h +++ b/include/config/Config.h @@ -28,6 +28,9 @@ class Config { const std::optional& getDebugPath() const { return debugPackage; } const std::optional& getFailureLogPath() const { return failureLogPath; }; + // Return the path which the JSON config lives in + const fs::path&getConfigDirPath() const { return configDirPath; } + // Non optional config variables const fs::path&getTestDirPath() const { return testDirPath; } @@ -64,7 +67,7 @@ class Config { std::optional failureLogPath; std::optional debugPackage; - fs::path testDirPath; + fs::path testDirPath, configDirPath; // Option file maps. PathMap executables; diff --git a/include/testharness/TestHarness.h b/include/testharness/TestHarness.h index ac32e329..89d5ae11 100644 --- a/include/testharness/TestHarness.h +++ b/include/testharness/TestHarness.h @@ -28,7 +28,17 @@ class TestHarness { TestHarness() = delete; // Construct the Tester with a parsed JSON file. - TestHarness(const Config& cfg) : cfg(cfg), results() { findTests(); } + TestHarness(const Config& cfg) + : cfg(cfg), + results() + { + // Create temporary dir for test and toolchain files + tmpPath = fs::path(cfg.getConfigDirPath() / "tmp"); + fs::create_directory(tmpPath); + + // Find tests + findTests(); + } // Returns true if any tests failed, false otherwise. bool runTests(); @@ -52,6 +62,9 @@ class TestHarness { // let derived classes find tests. void findTests(); + // Create a local tmp path for every test run + fs::path tmpPath; + private: // The results of the tests. ResultManager results; diff --git a/include/tests/TestFile.h b/include/tests/TestFile.h index 817baf84..95fdab8f 100644 --- a/include/tests/TestFile.h +++ b/include/tests/TestFile.h @@ -7,6 +7,7 @@ #include #include #include +#include namespace fs = std::filesystem; @@ -25,34 +26,32 @@ class TestParser; class TestFile { public: TestFile() = delete; - // construct Testfile from path to .test file. - TestFile(const fs::path& path); + TestFile(const fs::path& path, const fs::path& tmpPath); ~TestFile(); uint64_t id; - // getters - fs::path getTestPath() const { return testPath; } - fs::path getInsPath() const { return insPath; } - fs::path getOutPath() const { return outPath; } + // Test path getters + const fs::path& getTestPath() const { return testPath; } + const fs::path& getOutPath() const { return outPath; } + const fs::path& getInsPath() const { return insPath; } + + // Test state getters ParseError getParseError() const { return errorState; } std::string getParseErrorMsg() const; double getElapsedTime() const { return elapsedTime; } bool didError() const { return errorState != ParseError::NoError; } - // setters - void setTestPath(fs::path path) { testPath = path; } + // Test path getters void setInsPath(fs::path path) { insPath = path; } void setOutPath(fs::path path) { outPath = path; } - void getParseError(ParseError error) { errorState = error; } + + // Test path setters + void setParseError(ParseError error) { errorState = error; } void setParseErrorMsg(std::string msg) { errorMsg = msg; } void setElapsedTime(double elapsed) { elapsedTime = elapsed; } - // if test has any input and if test uses input file specifically - bool usesInputStream{false}, usesInputFile{false}; - bool usesOutStream{false}, usesOutFile{false}; - friend class TestParser; protected: @@ -60,7 +59,9 @@ class TestFile { private: // Path for the test, ins and out files - fs::path testPath, insPath, outPath; + fs::path testPath; + fs::path outPath; + fs::path insPath; // Test file breaks some convention or was unable to parse directives. ParseError errorState{ParseError::NoError}; diff --git a/include/tests/TestParser.h b/include/tests/TestParser.h index a72973d3..6614042a 100644 --- a/include/tests/TestParser.h +++ b/include/tests/TestParser.h @@ -5,6 +5,7 @@ #include "TestFile.h" #include #include +#include namespace fs = std::filesystem; @@ -42,8 +43,7 @@ class TestParser { // helper method to insert a newline prefixed line to a file void insLineToFile(fs::path filePath, std::string line, bool firstInsert); - // methods below look for INPUT, CHECK, INPUT_FILE, CHECK_FILE directive in - // a lines + // identifiy for INPUT, CHECK, INPUT_FILE or CHECK_FILE directive in a line ParseError matchInputDirective(std::string& line); ParseError matchCheckDirective(std::string& line); ParseError matchInputFileDirective(std::string& line); diff --git a/src/config/Config.cpp b/src/config/Config.cpp index 15221ad0..621533fe 100644 --- a/src/config/Config.cpp +++ b/src/config/Config.cpp @@ -55,6 +55,11 @@ Config::Config(int argc, char** argv) : timeout(2l) { JSON json; jsonFile >> json; + // Set the config path from the full path of the config file + configDirPath = fs::path(configFilePath).parent_path(); + if (!fs::exists(configDirPath)) + throw std::runtime_error("Can not find the directory of the config: " + configDirPath.string()); + // Make sure an in and out dir were provided. ensureContains(json, "testDir"); std::string testDirStr = json["testDir"]; diff --git a/src/testharness/TestHarness.cpp b/src/testharness/TestHarness.cpp index a6bc59c5..8ee78732 100644 --- a/src/testharness/TestHarness.cpp +++ b/src/testharness/TestHarness.cpp @@ -146,7 +146,7 @@ bool hasTestFiles(const fs::path& path) { } void TestHarness::addTestFileToSubPackage(SubPackage& subPackage, const fs::path& file) { - auto testfile = std::make_unique(file); + auto testfile = std::make_unique(file, tmpPath); TestParser parser(testfile.get()); diff --git a/src/tests/TestFile.cpp b/src/tests/TestFile.cpp index c91d180b..da9936e1 100644 --- a/src/tests/TestFile.cpp +++ b/src/tests/TestFile.cpp @@ -14,27 +14,24 @@ namespace tester { uint64_t TestFile::nextId = 0; -TestFile::TestFile(const fs::path& path) : id(++nextId), testPath(path) { +TestFile::TestFile(const fs::path& path, const fs::path& tmpPath) + : testPath(path) { - // create a unique temporary file to use as the inputs stream path - std::string fileInsPath = stripFileExtension(testPath.filename()); - insPath = fs::temp_directory_path() / (fileInsPath + std::to_string(id) + ".ins"); - outPath = fs::temp_directory_path() / (fileInsPath + std::to_string(id) + ".out"); + setInsPath(tmpPath / std::to_string(nextId) / "test.ins"); + setOutPath(tmpPath / std::to_string(nextId) / "test.ins"); + + std::cout << "Use tmp path: " << tmpPath << std::endl; + std::cout << "Out path: " << outPath << std::endl; + std::cout << "In path: " << insPath << std::endl; - std::ofstream makeInsFile(insPath); - std::ofstream makeOutFile(outPath); - - // closing creates the files - makeInsFile.close(); - makeOutFile.close(); + nextId++; } TestFile::~TestFile() { - // clean up allocated resources on Testfile de-allocation - if (usesInputStream && !usesInputFile) { + if (fs::exists(insPath)) { fs::remove(insPath); } - if (usesOutStream && !usesOutFile) { + if (fs::exists(outPath)) { fs::remove(outPath); } } diff --git a/src/tests/TestParser.cpp b/src/tests/TestParser.cpp index d95e2df6..4cda6012 100644 --- a/src/tests/TestParser.cpp +++ b/src/tests/TestParser.cpp @@ -61,14 +61,10 @@ ParseError TestParser::matchInputDirective(std::string& line) { size_t findIdx = line.find(Directive::INPUT); std::string inputLine = line.substr(findIdx + Directive::INPUT.length()); - - try { - insLineToFile(testfile->getInsPath(), inputLine, !foundInput); - } catch (...) { - return ParseError::FileError; - } - + + insLineToFile(testfile->getInsPath(), inputLine, foundInput); foundInput = true; + return ParseError::NoError; } @@ -85,14 +81,10 @@ ParseError TestParser::matchCheckDirective(std::string& line) { size_t findIdx = line.find(Directive::CHECK); std::string checkLine = line.substr(findIdx + Directive::CHECK.length()); - - try { - insLineToFile(testfile->getOutPath(), checkLine, !foundCheck); - } catch (...) { - return ParseError::FileError; - } - + + insLineToFile(testfile->getOutPath(), checkLine, foundCheck); foundCheck = true; + return ParseError::NoError; } @@ -107,13 +99,31 @@ ParseError TestParser::matchInputFileDirective(std::string& line) { if (foundInput) return ParseError::DirectiveConflict; - PathOrError pathOrError = parsePathFromLine(line, Directive::INPUT_FILE); - if (std::holds_alternative(pathOrError)) { - testfile->setInsPath(std::get(pathOrError)); + PathOrError inputFilePath = parsePathFromLine(line, Directive::INPUT_FILE); + if (std::holds_alternative(inputFilePath)) { + + // Extract the path from the variant + std::filesystem::path path = std::get(inputFilePath); + + // Open the supplied check file and read into + std::ifstream sourceFile(path, std::ios::binary); + std::ofstream destFile(testfile->getInsPath(), std::ios::binary); + if (!destFile) { + return ParseError::FileError; + } else if (!sourceFile) { + return ParseError::FileError; + } + + destFile << sourceFile.rdbuf(); + + if (sourceFile.fail() || destFile.fail()) { + return ParseError::FileError; + } + foundInputFile = true; return ParseError::NoError; } - return std::get(pathOrError); + return std::get(inputFilePath); } /** @@ -163,7 +173,7 @@ ParseError TestParser::matchDirectives(std::string& line) { * contained with in a comment. Use comment state in class instance to track. */ void TestParser::trackCommentState(std::string& line) { -std::string result; + std::string result; inLineComment = false; // reset line comment for (unsigned int i = 0; i < line.length(); i++) { @@ -217,19 +227,13 @@ void TestParser::parse() { if (!line.empty()) { ParseError error = matchDirectives(line); if (error != ParseError::NoError) { - testfile->getParseError(error); + testfile->setParseError(error); testfile->setParseErrorMsg("Generic Error"); break; } } } - // Set final flags to update test state - testfile->usesInputStream = (foundInput || foundInputFile); - testfile->usesInputFile = (foundInputFile); - testfile->usesOutStream = (foundCheck || foundCheckFile); - testfile->usesOutFile = foundCheckFile; - testFileStream.close(); } diff --git a/src/toolchain/Command.cpp b/src/toolchain/Command.cpp index 7e12686c..0eac32e4 100644 --- a/src/toolchain/Command.cpp +++ b/src/toolchain/Command.cpp @@ -80,7 +80,7 @@ void becomeCommand(const std::string& exe, int outFileStatus = redirectStdStream(output.c_str(), O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR, STDOUT_FILENO); int errorFileStatus = redirectStdStream(error.c_str(), O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR, STDERR_FILENO); int inFileStatus = !input.empty() ? redirectStdStream(input.c_str(), O_RDONLY, 0, STDIN_FILENO) : 0; - + // If opening any of the supplied output, input, or error files failed, raise here. if (outFileStatus == -1 || errorFileStatus == -1 || inFileStatus == -1) { perror("dup2"); @@ -106,6 +106,10 @@ void runCommand(std::promise& promise, std::atomic_bool& killVar, const std::string& error, const std::string& runtime) { + + std::cerr << "Made out file: " << input << std::endl; + std::cerr << "Made error file: " << output << std::endl; + pid_t childId = fork(); // We're the child process, we want to replace our process image with the From 85ae890f61fd45ce5b4ebf6971b0f582d6cf73c9 Mon Sep 17 00:00:00 2001 From: Justin Date: Mon, 16 Sep 2024 18:55:53 -0600 Subject: [PATCH 2/5] Ensure tmp files get created and deleted --- src/tests/TestFile.cpp | 38 ++++++++++++++++++++++++++++++++------ 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/src/tests/TestFile.cpp b/src/tests/TestFile.cpp index da9936e1..a32fc690 100644 --- a/src/tests/TestFile.cpp +++ b/src/tests/TestFile.cpp @@ -17,22 +17,48 @@ uint64_t TestFile::nextId = 0; TestFile::TestFile(const fs::path& path, const fs::path& tmpPath) : testPath(path) { - setInsPath(tmpPath / std::to_string(nextId) / "test.ins"); - setOutPath(tmpPath / std::to_string(nextId) / "test.ins"); - - std::cout << "Use tmp path: " << tmpPath << std::endl; - std::cout << "Out path: " << outPath << std::endl; - std::cout << "In path: " << insPath << std::endl; + fs::path testDir = tmpPath / std::to_string(nextId); + setInsPath(testDir / "test.ins"); + setOutPath(testDir / "test.out"); + try { + // Create tmp directory if it doesn't exist + std::cout << "Attempting to create directory: " << testDir << std::endl; + if (!fs::exists(testDir)) { + if (!fs::create_directories(testDir)) { + throw std::runtime_error("Failed to create directory: " + testDir.string()); + } + } + // Create the temporary input and ouput files + std::ofstream createInsFile(insPath); + std::ofstream createOutFile(outPath); + if (!createInsFile) { + throw std::runtime_error("Failed to create input file: " + insPath.string()); + } + if (!createOutFile) { + throw std::runtime_error("Failed to create output file: " + outPath.string()); + } + createInsFile.close(); + createOutFile.close(); + + } catch (const fs::filesystem_error& e) { + throw std::runtime_error("Filesystem error: " + std::string(e.what())); + } catch (const std::exception& e) { + throw std::runtime_error("Error in TestFile constructor: " + std::string(e.what())); + } nextId++; } TestFile::~TestFile() { if (fs::exists(insPath)) { + // Remove temporary input stream file fs::remove(insPath); } if (fs::exists(outPath)) { + // Remove the tenmporary testfile directory and the expected out + fs::path testfileDir = outPath.parent_path(); fs::remove(outPath); + fs::remove(testfileDir); } } From 0ecfec00a5b8b3fc9cb856d07849b4b635e0f263 Mon Sep 17 00:00:00 2001 From: Justin Date: Tue, 17 Sep 2024 18:34:21 -0600 Subject: [PATCH 3/5] Make testfile ids atomic and create ins and out files on demand --- include/testharness/TestHarness.h | 8 ++-- include/tests/TestFile.h | 11 ++++-- include/toolchain/Command.h | 6 +-- include/toolchain/ExecutionState.h | 6 ++- src/testharness/TestHarness.cpp | 2 +- src/tests/TestFile.cpp | 47 +++++++++------------- src/tests/TestParser.cpp | 62 ++++++++++++++++-------------- src/tests/TestRunning.cpp | 1 - src/toolchain/Command.cpp | 34 +++++++++------- src/toolchain/ToolChain.cpp | 4 +- 10 files changed, 95 insertions(+), 86 deletions(-) diff --git a/include/testharness/TestHarness.h b/include/testharness/TestHarness.h index 89d5ae11..40529b6c 100644 --- a/include/testharness/TestHarness.h +++ b/include/testharness/TestHarness.h @@ -33,8 +33,8 @@ class TestHarness { results() { // Create temporary dir for test and toolchain files - tmpPath = fs::path(cfg.getConfigDirPath() / "tmp"); - fs::create_directory(tmpPath); + testArtifactsPath = fs::path(cfg.getConfigDirPath() / ".test-artifacts"); + fs::create_directory(testArtifactsPath); // Find tests findTests(); @@ -62,8 +62,8 @@ class TestHarness { // let derived classes find tests. void findTests(); - // Create a local tmp path for every test run - fs::path tmpPath; + // Create a local tmp path for ephemeral test input, output and toolchain files + fs::path testArtifactsPath; private: // The results of the tests. diff --git a/include/tests/TestFile.h b/include/tests/TestFile.h index 95fdab8f..4c1723e1 100644 --- a/include/tests/TestFile.h +++ b/include/tests/TestFile.h @@ -8,6 +8,7 @@ #include #include #include +#include namespace fs = std::filesystem; @@ -27,10 +28,10 @@ class TestFile { public: TestFile() = delete; // construct Testfile from path to .test file. - TestFile(const fs::path& path, const fs::path& tmpPath); + TestFile(const fs::path& path, const fs::path& artifactDir); ~TestFile(); - uint64_t id; + uint64_t getId() const { return id; } // Test path getters const fs::path& getTestPath() const { return testPath; } @@ -54,10 +55,14 @@ class TestFile { friend class TestParser; +private: + static uint64_t generateId(); + protected: - static uint64_t nextId; + static std::atomic nextId; private: + uint64_t id; // Path for the test, ins and out files fs::path testPath; fs::path outPath; diff --git a/include/toolchain/Command.h b/include/toolchain/Command.h index 4ddfe392..a742d3c6 100644 --- a/include/toolchain/Command.h +++ b/include/toolchain/Command.h @@ -58,15 +58,13 @@ class Command { std::string name; fs::path exePath; fs::path runtimePath; + fs::path tmpPath; std::vector args; - + // Every command produces a file descriptor for each of these paths fs::path errPath; fs::path outPath; - // The command can supply a output file to use instead of stdout/err - std::optional outputFile; - // Uses runtime and uses input stream. bool usesRuntime, usesInStr; diff --git a/include/toolchain/ExecutionState.h b/include/toolchain/ExecutionState.h index 1de14f46..cfc836d9 100644 --- a/include/toolchain/ExecutionState.h +++ b/include/toolchain/ExecutionState.h @@ -16,8 +16,10 @@ class ExecutionInput { // Creates input to a subprocess execution. ExecutionInput(fs::path inputPath, fs::path inputStreamPath, fs::path testedExecutable, fs::path testedRuntime) - : inputPath(std::move(inputPath)), inputStreamPath(std::move(inputStreamPath)), - testedExecutable(std::move(testedExecutable)), testedRuntime(std::move(testedRuntime)) {} + : inputPath(std::move(inputPath)), + inputStreamPath(std::move(inputStreamPath)), + testedExecutable(std::move(testedExecutable)), + testedRuntime(std::move(testedRuntime)) {} // Gets input file. const fs::path& getInputFile() const { return inputPath; } diff --git a/src/testharness/TestHarness.cpp b/src/testharness/TestHarness.cpp index 8ee78732..694dd04d 100644 --- a/src/testharness/TestHarness.cpp +++ b/src/testharness/TestHarness.cpp @@ -146,7 +146,7 @@ bool hasTestFiles(const fs::path& path) { } void TestHarness::addTestFileToSubPackage(SubPackage& subPackage, const fs::path& file) { - auto testfile = std::make_unique(file, tmpPath); + auto testfile = std::make_unique(file, testArtifactsPath); TestParser parser(testfile.get()); diff --git a/src/tests/TestFile.cpp b/src/tests/TestFile.cpp index a32fc690..1129b032 100644 --- a/src/tests/TestFile.cpp +++ b/src/tests/TestFile.cpp @@ -1,6 +1,8 @@ #include "tests/TestFile.h" #include "tests/TestParser.h" +static uint64_t nextId = 0; + namespace { std::string stripFileExtension(const std::string& str) { @@ -12,53 +14,42 @@ std::string stripFileExtension(const std::string& str) { namespace tester { -uint64_t TestFile::nextId = 0; +// Initialize the static id to zero +std::atomic TestFile::nextId(0); + +uint64_t TestFile::generateId() { + return nextId.fetch_add(1, std::memory_order_relaxed); +} -TestFile::TestFile(const fs::path& path, const fs::path& tmpPath) - : testPath(path) { +TestFile::TestFile(const fs::path& path, const fs::path& artifactDir) + : id(generateId()), testPath(path) { - fs::path testDir = tmpPath / std::to_string(nextId); - setInsPath(testDir / "test.ins"); - setOutPath(testDir / "test.out"); + std::string testName = path.stem(); + setInsPath(artifactDir / fs::path(testName + std::to_string(id) + ".ins")); + setOutPath(artifactDir / fs::path(testName + std::to_string(id) + ".out")); try { // Create tmp directory if it doesn't exist - std::cout << "Attempting to create directory: " << testDir << std::endl; - if (!fs::exists(testDir)) { - if (!fs::create_directories(testDir)) { - throw std::runtime_error("Failed to create directory: " + testDir.string()); + if (!fs::exists(artifactDir)) { + if (!fs::create_directories(artifactDir)) { + throw std::runtime_error("Failed to create directory: " + artifactDir.string()); } } - // Create the temporary input and ouput files - std::ofstream createInsFile(insPath); - std::ofstream createOutFile(outPath); - if (!createInsFile) { - throw std::runtime_error("Failed to create input file: " + insPath.string()); - } - if (!createOutFile) { - throw std::runtime_error("Failed to create output file: " + outPath.string()); - } - createInsFile.close(); - createOutFile.close(); - } catch (const fs::filesystem_error& e) { throw std::runtime_error("Filesystem error: " + std::string(e.what())); - } catch (const std::exception& e) { - throw std::runtime_error("Error in TestFile constructor: " + std::string(e.what())); - } - nextId++; + } } TestFile::~TestFile() { + + std::cout << "Calling Destructor...\n"; if (fs::exists(insPath)) { // Remove temporary input stream file fs::remove(insPath); } if (fs::exists(outPath)) { // Remove the tenmporary testfile directory and the expected out - fs::path testfileDir = outPath.parent_path(); fs::remove(outPath); - fs::remove(testfileDir); } } diff --git a/src/tests/TestParser.cpp b/src/tests/TestParser.cpp index 4cda6012..ce81a720 100644 --- a/src/tests/TestParser.cpp +++ b/src/tests/TestParser.cpp @@ -14,6 +14,26 @@ bool fullyContains(const std::string& str, const std::string& substr) { return str.substr(pos, substr.length()) == substr; } +ParseError copyFile(const fs::path& from, const fs::path& to) { + + // Open the files to operate upon + std::ifstream sourceFile(from, std::ios::binary); + std::ofstream destFile(to, std::ios::binary); + + // Check for errors opening + if (!destFile || !sourceFile) { + return ParseError::FileError; + } + + // Write the contents and check for errors + destFile << sourceFile.rdbuf(); + if (sourceFile.fail() || destFile.fail()) { + return ParseError::FileError; + } + + return ParseError::NoError; +} + void TestParser::insLineToFile(fs::path filePath, std::string line, bool firstInsert) { // open in append mode since otherwise multi-line checks and inputs would // over-write themselves. @@ -99,31 +119,15 @@ ParseError TestParser::matchInputFileDirective(std::string& line) { if (foundInput) return ParseError::DirectiveConflict; - PathOrError inputFilePath = parsePathFromLine(line, Directive::INPUT_FILE); - if (std::holds_alternative(inputFilePath)) { - - // Extract the path from the variant - std::filesystem::path path = std::get(inputFilePath); - - // Open the supplied check file and read into - std::ifstream sourceFile(path, std::ios::binary); - std::ofstream destFile(testfile->getInsPath(), std::ios::binary); - if (!destFile) { - return ParseError::FileError; - } else if (!sourceFile) { - return ParseError::FileError; - } - - destFile << sourceFile.rdbuf(); - - if (sourceFile.fail() || destFile.fail()) { - return ParseError::FileError; - } - + PathOrError path = parsePathFromLine(line, Directive::INPUT_FILE); + if (std::holds_alternative(path)) { + // Copy the input file referenced into the testfiles ephemeral ins file + auto inputPath = std::get(path); + copyFile(inputPath, testfile->getInsPath()); foundInputFile = true; - return ParseError::NoError; } - return std::get(inputFilePath); + + return std::get(path); } /** @@ -137,14 +141,16 @@ ParseError TestParser::matchCheckFileDirective(std::string& line) { if (foundCheck) return ParseError::DirectiveConflict; - PathOrError pathOrError = parsePathFromLine(line, Directive::CHECK_FILE); - if (std::holds_alternative(pathOrError)) { - testfile->setOutPath(std::get(pathOrError)); - foundCheckFile = true; + PathOrError path = parsePathFromLine(line, Directive::CHECK_FILE); + if (std::holds_alternative(path)) { + // Copy the input file referenced into the testfiles ephemeral ins file + auto outputPath = std::get(path); + copyFile(outputPath, testfile->getOutPath()); + foundCheckFile= true; return ParseError::NoError; } - return std::get(pathOrError); + return std::get(path); } /** diff --git a/src/tests/TestRunning.cpp b/src/tests/TestRunning.cpp index a4ac1015..47a9acda 100644 --- a/src/tests/TestRunning.cpp +++ b/src/tests/TestRunning.cpp @@ -193,7 +193,6 @@ TestResult runTest(TestFile* test, const ToolChain& toolChain, const Config& cfg const fs::path testPath = test->getTestPath(); const fs::path expOutPath = test->getOutPath(); - const fs::path insPath = test->getInsPath(); fs::path genOutPath; std::string genErrorString, expErrorString, diffString; diff --git a/src/toolchain/Command.cpp b/src/toolchain/Command.cpp index 0eac32e4..9fbac551 100644 --- a/src/toolchain/Command.cpp +++ b/src/toolchain/Command.cpp @@ -105,8 +105,8 @@ void runCommand(std::promise& promise, std::atomic_bool& killVar, const std::string& output, const std::string& error, const std::string& runtime) { - - + + std::cerr << "Running Command: " << exe << std::endl; std::cerr << "Made out file: " << input << std::endl; std::cerr << "Made error file: " << output << std::endl; @@ -178,6 +178,9 @@ namespace tester { Command::Command(const JSON& step, int64_t timeout) : usesRuntime(false), usesInStr(false), timeout(timeout) { + + fs::path tmpPath = "./tmp"; + // Make sure the step has all of the values needed for construction. ensureContains(step, "stepName"); ensureContains(step, "executablePath"); @@ -188,20 +191,23 @@ Command::Command(const JSON& step, int64_t timeout) for (std::string arg : step["arguments"]) args.push_back(arg); - // If no output path is supplied by default, temporaries are created to capture stdout and stderr. - std::string output_name = std::string(step["stepName"]) + ".stdout"; - std::string error_name = std::string(step["stepName"]) + ".stderr"; - outPath = fs::temp_directory_path() / output_name; - errPath = fs::temp_directory_path() / error_name; - // Set the executable path std::string path = step["executablePath"]; exePath = fs::path(path); - // Allow override of stdout path - if (doesContain(step, "output")) - outputFile = fs::path(step["output"]); + // Allow override of stdout path with output property + if (doesContain(step, "output")) { + outPath = tmpPath / fs::path(step["output"]); + } else { + std::string output_name = std::string(step["stepName"]) + ".stdout"; + outPath = tmpPath / output_name; + } + std::cout << "Created commadn with outpath: " << outPath << std::endl; + // Always create a stderr path + std::string error_name = std::string(step["stepName"]) + ".stderr"; + errPath = tmpPath / error_name; + // Do we use an input stream file? if (doesContain(step, "usesInStr")) usesInStr = step["usesInStr"]; @@ -212,13 +218,12 @@ Command::Command(const JSON& step, int64_t timeout) // Do we allow errors? if (doesContain(step, "allowError")) - allowError = step["allowError"]; + allowError = step["allowError"]; } ExecutionOutput Command::execute(const ExecutionInput& ei) const { // Create our output context. - fs::path out = outputFile.has_value() ? *outputFile : outPath; - ExecutionOutput eo(out, errPath); + ExecutionOutput eo(outPath, errPath); // Always remove old output files so we know if a new one was created std::error_code ec; @@ -230,6 +235,7 @@ ExecutionOutput Command::execute(const ExecutionInput& ei) const { for (const std::string& arg : args) trueArgs.emplace_back(resolveArg(ei, eo, arg).string()); + std::cout << "OUT PATH: " << outPath << std::endl; // Get the runtime path and standard out file, the things used in setting up // the execution of the command. std::string runtimeStr = usesRuntime ? ei.getTestedRuntime().string() : ""; diff --git a/src/toolchain/ToolChain.cpp b/src/toolchain/ToolChain.cpp index 70441512..8cb90714 100644 --- a/src/toolchain/ToolChain.cpp +++ b/src/toolchain/ToolChain.cpp @@ -34,7 +34,9 @@ ExecutionOutput ToolChain::build(TestFile* test) const { eo.setIsErrorTest(true); return eo; } - + + // The input for the next command is the output of the previous command, along with + // the same input stream, tested executable and runtime, which is shared for all commands. ei = ExecutionInput(eo.getOutputFile(), ei.getInputStreamFile(), ei.getTestedExecutable(), ei.getTestedRuntime()); } From 239948a9f1e207f69edbd132888a8939fc0fa2e3 Mon Sep 17 00:00:00 2001 From: Justin Date: Tue, 17 Sep 2024 19:21:32 -0600 Subject: [PATCH 4/5] Improve CHECK and INPUT line insertion --- src/tests/TestFile.cpp | 25 +++++++++++++++---------- src/tests/TestParser.cpp | 25 +++++++++++++++++-------- 2 files changed, 32 insertions(+), 18 deletions(-) diff --git a/src/tests/TestFile.cpp b/src/tests/TestFile.cpp index 1129b032..59b35188 100644 --- a/src/tests/TestFile.cpp +++ b/src/tests/TestFile.cpp @@ -17,6 +17,7 @@ namespace tester { // Initialize the static id to zero std::atomic TestFile::nextId(0); +// An atoimc uint64_t TestFile::generateId() { return nextId.fetch_add(1, std::memory_order_relaxed); } @@ -25,8 +26,12 @@ TestFile::TestFile(const fs::path& path, const fs::path& artifactDir) : id(generateId()), testPath(path) { std::string testName = path.stem(); - setInsPath(artifactDir / fs::path(testName + std::to_string(id) + ".ins")); - setOutPath(artifactDir / fs::path(testName + std::to_string(id) + ".out")); + setInsPath(artifactDir / fs::path(testName + '-' + std::to_string(id) + ".ins")); + setOutPath(artifactDir / fs::path(testName + '-' + std::to_string(id) + ".out")); + + std::cout << "Creating file: " << testName << std::endl; + std::cout << "INS: " << getInsPath() << std::endl; + std::cout << "OUT: " << getInsPath() << std::endl; try { // Create tmp directory if it doesn't exist @@ -43,14 +48,14 @@ TestFile::TestFile(const fs::path& path, const fs::path& artifactDir) TestFile::~TestFile() { std::cout << "Calling Destructor...\n"; - if (fs::exists(insPath)) { - // Remove temporary input stream file - fs::remove(insPath); - } - if (fs::exists(outPath)) { - // Remove the tenmporary testfile directory and the expected out - fs::remove(outPath); - } + // if (fs::exists(insPath)) { + // // Remove temporary input stream file + // fs::remove(insPath); + // } + // if (fs::exists(outPath)) { + // // Remove the tenmporary testfile directory and the expected out + // fs::remove(outPath); + // } } std::string TestFile::getParseErrorMsg() const { diff --git a/src/tests/TestParser.cpp b/src/tests/TestParser.cpp index ce81a720..4b94234b 100644 --- a/src/tests/TestParser.cpp +++ b/src/tests/TestParser.cpp @@ -35,13 +35,22 @@ ParseError copyFile(const fs::path& from, const fs::path& to) { } void TestParser::insLineToFile(fs::path filePath, std::string line, bool firstInsert) { - // open in append mode since otherwise multi-line checks and inputs would - // over-write themselves. - std::ofstream out(filePath, std::ios::app); - if (!firstInsert) { - out << "\n"; + + // Set the mode to open the file determined by first insert + std::ios_base::openmode mode; + if (firstInsert) { + mode = std::ios::out | std::ios::trunc; + } else { + mode = std::ios::app; } + + // Open the file, write the contents. + std::ofstream out(filePath, mode); out << line; + if (!firstInsert) { + out << "\n"; + } + out.close(); } /** @@ -82,7 +91,7 @@ ParseError TestParser::matchInputDirective(std::string& line) { size_t findIdx = line.find(Directive::INPUT); std::string inputLine = line.substr(findIdx + Directive::INPUT.length()); - insLineToFile(testfile->getInsPath(), inputLine, foundInput); + insLineToFile(testfile->getInsPath(), inputLine, !foundInput); foundInput = true; return ParseError::NoError; @@ -98,11 +107,11 @@ ParseError TestParser::matchCheckDirective(std::string& line) { return ParseError::NoError; if (foundCheckFile) return ParseError::DirectiveConflict; - + size_t findIdx = line.find(Directive::CHECK); std::string checkLine = line.substr(findIdx + Directive::CHECK.length()); - insLineToFile(testfile->getOutPath(), checkLine, foundCheck); + insLineToFile(testfile->getOutPath(), checkLine, !foundCheck); foundCheck = true; return ParseError::NoError; From 14f452848dd1de2be05b95e27ff3e3e23474a17f Mon Sep 17 00:00:00 2001 From: Justin Date: Tue, 17 Sep 2024 22:51:35 -0600 Subject: [PATCH 5/5] Create all ephemeral files in .test-artifacts --- include/toolchain/Command.h | 2 +- include/toolchain/ExecutionState.h | 9 +++++ src/main.cpp | 2 +- src/tests/TestFile.cpp | 53 ++++++++++++++++---------- src/tests/TestParser.cpp | 25 +++++------- src/tests/TestRunning.cpp | 19 ++++++++-- src/toolchain/Command.cpp | 61 ++++++++++++++++++++---------- 7 files changed, 108 insertions(+), 63 deletions(-) diff --git a/include/toolchain/Command.h b/include/toolchain/Command.h index a742d3c6..e6b3e20e 100644 --- a/include/toolchain/Command.h +++ b/include/toolchain/Command.h @@ -32,7 +32,7 @@ class Command { Command(const Command& command) = default; // Destructor for removing temporary files - ~Command() {} + ~Command(); // Execute the command. ExecutionOutput execute(const ExecutionInput& ei) const; diff --git a/include/toolchain/ExecutionState.h b/include/toolchain/ExecutionState.h index cfc836d9..588b3a56 100644 --- a/include/toolchain/ExecutionState.h +++ b/include/toolchain/ExecutionState.h @@ -1,6 +1,7 @@ #ifndef TESTER_EXECUTION_STATE_H #define TESTER_EXECUTION_STATE_H +#include #include #include namespace fs = std::filesystem; @@ -21,6 +22,10 @@ class ExecutionInput { testedExecutable(std::move(testedExecutable)), testedRuntime(std::move(testedRuntime)) {} + ~ExecutionInput() { + // std::cout << "Destory execution input: " << inputPath << std::endl; + } + // Gets input file. const fs::path& getInputFile() const { return inputPath; } @@ -52,6 +57,10 @@ class ExecutionOutput { errPath(std::move(errPath)), rv(0), elapsedTime(0), hasElapsed(false), isErrorTest(false) {} + ~ExecutionOutput() { + // std::cout << "Destory execution output: " << outPath << std::endl; + } + // Gets output file. fs::path getOutputFile() const { return outPath; } fs::path getErrorFile() const { return errPath; } diff --git a/src/main.cpp b/src/main.cpp index 9f940cef..819cb736 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -34,7 +34,7 @@ int main(int argc, char** argv) { // Free resources } catch (const std::runtime_error& e) { - std::cout << "Test harness error: " << e.what() << '\n'; + std::cout << e.what() << '\n'; return 1; } diff --git a/src/tests/TestFile.cpp b/src/tests/TestFile.cpp index 59b35188..35eb714e 100644 --- a/src/tests/TestFile.cpp +++ b/src/tests/TestFile.cpp @@ -25,21 +25,28 @@ uint64_t TestFile::generateId() { TestFile::TestFile(const fs::path& path, const fs::path& artifactDir) : id(generateId()), testPath(path) { - std::string testName = path.stem(); - setInsPath(artifactDir / fs::path(testName + '-' + std::to_string(id) + ".ins")); - setOutPath(artifactDir / fs::path(testName + '-' + std::to_string(id) + ".out")); - - std::cout << "Creating file: " << testName << std::endl; - std::cout << "INS: " << getInsPath() << std::endl; - std::cout << "OUT: " << getInsPath() << std::endl; - try { - // Create tmp directory if it doesn't exist + // Create .test-artifacts if it doesn't exist if (!fs::exists(artifactDir)) { - if (!fs::create_directories(artifactDir)) { - throw std::runtime_error("Failed to create directory: " + artifactDir.string()); - } + fs::create_directories(artifactDir); + } + + // create .test-artifacts/testfiles if it doesn't exist + fs::path testArtifactsDir = artifactDir / "testfiles"; + if (!fs::exists(testArtifactsDir)) { + fs::create_directories(testArtifactsDir); } + + std::string testName = path.stem(); + fs::path basePath = testArtifactsDir / fs::path(testName + '-' + std::to_string(id)); + + setInsPath(fs::path(basePath.string() + ".ins")); + setOutPath(fs::path(basePath.string() + ".out")); + + // std::cout << "Creating file: " << testName << std::endl; + // std::cout << "INS: " << getInsPath() << std::endl; + // std::cout << "OUT: " << getOutPath() << std::endl; + } catch (const fs::filesystem_error& e) { throw std::runtime_error("Filesystem error: " + std::string(e.what())); } @@ -47,15 +54,19 @@ TestFile::TestFile(const fs::path& path, const fs::path& artifactDir) TestFile::~TestFile() { - std::cout << "Calling Destructor...\n"; - // if (fs::exists(insPath)) { - // // Remove temporary input stream file - // fs::remove(insPath); - // } - // if (fs::exists(outPath)) { - // // Remove the tenmporary testfile directory and the expected out - // fs::remove(outPath); - // } + // std::cout << "Calling Destructor...\n"; + try { + if (fs::exists(insPath)) { + // Remove temporary input stream file + fs::remove(insPath); + } + if (fs::exists(outPath)) { + // Remove the tenmporary testfile directory and the expected out + fs::remove(outPath); + } + } catch (const std::exception& e) { + std::cerr << "Caught exception in destructor: "<< e.what() << std::endl; + } } std::string TestFile::getParseErrorMsg() const { diff --git a/src/tests/TestParser.cpp b/src/tests/TestParser.cpp index 4b94234b..d5f8c8fa 100644 --- a/src/tests/TestParser.cpp +++ b/src/tests/TestParser.cpp @@ -35,22 +35,14 @@ ParseError copyFile(const fs::path& from, const fs::path& to) { } void TestParser::insLineToFile(fs::path filePath, std::string line, bool firstInsert) { - - // Set the mode to open the file determined by first insert - std::ios_base::openmode mode; + if (firstInsert) { - mode = std::ios::out | std::ios::trunc; + std::ofstream out(filePath); + out << line; } else { - mode = std::ios::app; - } - - // Open the file, write the contents. - std::ofstream out(filePath, mode); - out << line; - if (!firstInsert) { - out << "\n"; - } - out.close(); + std::ofstream out(filePath, std::ios::app); + out << "\n" << line; + } } /** @@ -90,7 +82,7 @@ ParseError TestParser::matchInputDirective(std::string& line) { size_t findIdx = line.find(Directive::INPUT); std::string inputLine = line.substr(findIdx + Directive::INPUT.length()); - + insLineToFile(testfile->getInsPath(), inputLine, !foundInput); foundInput = true; @@ -110,7 +102,7 @@ ParseError TestParser::matchCheckDirective(std::string& line) { size_t findIdx = line.find(Directive::CHECK); std::string checkLine = line.substr(findIdx + Directive::CHECK.length()); - + insLineToFile(testfile->getOutPath(), checkLine, !foundCheck); foundCheck = true; @@ -134,6 +126,7 @@ ParseError TestParser::matchInputFileDirective(std::string& line) { auto inputPath = std::get(path); copyFile(inputPath, testfile->getInsPath()); foundInputFile = true; + return ParseError::NoError; } return std::get(path); diff --git a/src/tests/TestRunning.cpp b/src/tests/TestRunning.cpp index 47a9acda..2699f74f 100644 --- a/src/tests/TestRunning.cpp +++ b/src/tests/TestRunning.cpp @@ -149,10 +149,21 @@ void formatFileDump(const fs::path& testPath, const fs::path& expOutPath, const fs::path& genOutPath) { std::cout << "----- TestFile: "<< testPath.filename() << std::endl; dumpFile(testPath); - std::cout << "----- Expected Output (" << fs::file_size(expOutPath) << " bytes)" << std::endl; - dumpFile(expOutPath, true); - std::cout << "----- Generated Output (" << fs::file_size(genOutPath) << " bytes)" << std::endl; - dumpFile(genOutPath, true); + + if (fs::exists(expOutPath)) { + std::cout << "----- Expected Output (" << fs::file_size(expOutPath) << " bytes)" << std::endl; + dumpFile(expOutPath, true); + } else { + std::cout << "----- Expected Output: (0 bytes)" << std::endl; + dumpFile("/dev/null", true); + } + if (fs::exists(genOutPath)) { + std::cout << "----- Generated Output (" << fs::file_size(genOutPath) << " bytes)" << std::endl; + dumpFile(genOutPath, true); + } else { + std::cout << "----- Generated Output: (0 bytes)" << std::endl; + dumpFile("/dev/null", true); + } std::cout << "-----------------------" << std::endl; } diff --git a/src/toolchain/Command.cpp b/src/toolchain/Command.cpp index 9fbac551..01cbf60d 100644 --- a/src/toolchain/Command.cpp +++ b/src/toolchain/Command.cpp @@ -22,7 +22,7 @@ namespace { /// @brief Open the file with provided flags and mode. Redirect the file descriptor /// supplied by dup_fd to the file underlying file_str. int redirectStdStream(const std::string& file_str, int flags, mode_t mode, int dup_fd) { - + // Open the process int fd = open(file_str.c_str(), flags, mode); if (fd == -1) { @@ -78,13 +78,19 @@ void becomeCommand(const std::string& exe, // Open the supplied files and redirect FD of the current child process to them. int outFileStatus = redirectStdStream(output.c_str(), O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR, STDOUT_FILENO); - int errorFileStatus = redirectStdStream(error.c_str(), O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR, STDERR_FILENO); - int inFileStatus = !input.empty() ? redirectStdStream(input.c_str(), O_RDONLY, 0, STDIN_FILENO) : 0; + int errFileStatus = redirectStdStream(error.c_str(), O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR, STDERR_FILENO); + int insFileStatus = !input.empty() ? redirectStdStream(input.c_str(), O_RDONLY, 0, STDIN_FILENO) : 0; // If opening any of the supplied output, input, or error files failed, raise here. - if (outFileStatus == -1 || errorFileStatus == -1 || inFileStatus == -1) { - perror("dup2"); - exit(EXIT_FAILURE); + if (outFileStatus == -1) { + perror("dup2 failed to open output file"); + exit(EXIT_FAILURE); + } else if (errFileStatus == -1) { + perror("dup2 failed to open error file"); + exit(EXIT_FAILURE); + } else if (insFileStatus == -1) { + perror("dup2 failed to open input stream file"); + exit(EXIT_FAILURE); } // Replace ourselves with the command. @@ -105,11 +111,13 @@ void runCommand(std::promise& promise, std::atomic_bool& killVar, const std::string& output, const std::string& error, const std::string& runtime) { - + +#if defined(DEBUG) std::cerr << "Running Command: " << exe << std::endl; - std::cerr << "Made out file: " << input << std::endl; - std::cerr << "Made error file: " << output << std::endl; - + std::cerr << "Made out file: " << output << std::endl; + std::cerr << "Made error file: " << error << std::endl; + std::cerr << "Made input file: " << input << std::endl; +#endif pid_t childId = fork(); // We're the child process, we want to replace our process image with the @@ -179,7 +187,7 @@ namespace tester { Command::Command(const JSON& step, int64_t timeout) : usesRuntime(false), usesInStr(false), timeout(timeout) { - fs::path tmpPath = "./tmp"; + fs::path testArtifactsPath = "./.test-artifacts"; // Make sure the step has all of the values needed for construction. ensureContains(step, "stepName"); @@ -197,17 +205,16 @@ Command::Command(const JSON& step, int64_t timeout) // Allow override of stdout path with output property if (doesContain(step, "output")) { - outPath = tmpPath / fs::path(step["output"]); + outPath = testArtifactsPath / fs::path(step["output"]); } else { - std::string output_name = std::string(step["stepName"]) + ".stdout"; - outPath = tmpPath / output_name; + std::string outFileName = std::string(step["stepName"]) + ".stdout"; + outPath = testArtifactsPath / outFileName; } - std::cout << "Created commadn with outpath: " << outPath << std::endl; // Always create a stderr path - std::string error_name = std::string(step["stepName"]) + ".stderr"; - errPath = tmpPath / error_name; - + std::string errFileName = std::string(step["stepName"]) + ".stderr"; + errPath = testArtifactsPath / fs::path(errFileName); + // Do we use an input stream file? if (doesContain(step, "usesInStr")) usesInStr = step["usesInStr"]; @@ -218,10 +225,18 @@ Command::Command(const JSON& step, int64_t timeout) // Do we allow errors? if (doesContain(step, "allowError")) - allowError = step["allowError"]; + allowError = step["allowError"]; + + // std::cout << "Created command with outpath: " << outPath << std::endl; + // std::cout << "Created command with errPath: " << errPath << std::endl; +} + +Command::~Command() { + // std::cout << "Destroying command\n"; } ExecutionOutput Command::execute(const ExecutionInput& ei) const { + // Create our output context. ExecutionOutput eo(outPath, errPath); @@ -235,11 +250,17 @@ ExecutionOutput Command::execute(const ExecutionInput& ei) const { for (const std::string& arg : args) trueArgs.emplace_back(resolveArg(ei, eo, arg).string()); +#if defined(DEBUG) std::cout << "OUT PATH: " << outPath << std::endl; + std::cout << "ERR PATH: " << errPath << std::endl; + std::cout << "INS PATH: " << ei.getInputStreamFile() << std::endl; +#endif // Get the runtime path and standard out file, the things used in setting up // the execution of the command. std::string runtimeStr = usesRuntime ? ei.getTestedRuntime().string() : ""; - std::string inPathStr = usesInStr ? ei.getInputStreamFile().string() : ""; + std::string inPathStr = fs::exists(ei.getInputStreamFile()) && usesInStr + ? ei.getInputStreamFile().string() + : ""; std::string outPathStr = outPath.string(); std::string errPathStr = errPath.string();