Make testfile ids atomic and create ins and out files on demand

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.

include/tests/TestFile.h

@@ -8,6 +8,7 @@
 #include <string>
 #include <vector>
 #include <optional>
+#include <atomic>
 
 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<uint64_t> nextId;
 
 private:
+  uint64_t id;
   // Path for the test, ins and out files 
   fs::path testPath;
   fs::path outPath;

include/toolchain/Command.h

@@ -58,15 +58,13 @@ class Command {
   std::string name;
   fs::path exePath;
   fs::path runtimePath;
+  fs::path tmpPath;
   std::vector<std::string> 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<fs::path> outputFile;
-
   // Uses runtime and uses input stream.
   bool usesRuntime, usesInStr;
 

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; }

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<TestFile>(file, tmpPath);
+  auto testfile = std::make_unique<TestFile>(file, testArtifactsPath);
 
   TestParser parser(testfile.get());
 

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<uint64_t> 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);
   }
 }
 

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<fs::path>(inputFilePath)) {
-
-    // Extract the path from the variant
-    std::filesystem::path path = std::get<fs::path>(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<fs::path>(path)) {
+    // Copy the input file referenced into the testfiles ephemeral ins file 
+    auto inputPath = std::get<fs::path>(path);
+    copyFile(inputPath, testfile->getInsPath());
     foundInputFile = true;
-    return ParseError::NoError;
   }
-  return std::get<ParseError>(inputFilePath);
+
+  return std::get<ParseError>(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<fs::path>(pathOrError)) {
-    testfile->setOutPath(std::get<fs::path>(pathOrError));
-    foundCheckFile = true;
+  PathOrError path = parsePathFromLine(line, Directive::CHECK_FILE);
+  if (std::holds_alternative<fs::path>(path)) {
+    // Copy the input file referenced into the testfiles ephemeral ins file 
+    auto outputPath = std::get<fs::path>(path);
+    copyFile(outputPath, testfile->getOutPath());
+    foundCheckFile= true;
     return ParseError::NoError;
   }
 
-  return std::get<ParseError>(pathOrError);
+  return std::get<ParseError>(path);
 }
 
 /**

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;
 

src/toolchain/Command.cpp

@@ -105,8 +105,8 @@ void runCommand(std::promise<unsigned int>& 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() : "";

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());
   }