Validate that assigned subtasks in test groups are defined (#120)

Resolves #114

include/tcframe/spec/constraint/ConstraintSuite.hpp

@@ -1,5 +1,6 @@
 #pragma once
 
+#include <set>
 #include <tuple>
 #include <utility>
 #include <vector>
@@ -8,6 +9,7 @@
 #include "Subtask.hpp"
 
 using std::move;
+using std::set;
 using std::tie;
 using std::vector;
 
@@ -38,6 +40,16 @@ struct ConstraintSuite {
     bool operator==(const ConstraintSuite& o) const {
         return tie(constraints_, multipleTestCasesConstraints_) == tie(o.constraints_, multipleTestCasesConstraints_);
     }
+
+    set<int> getDefinedSubtaskIds() const {
+        set<int> definedSubtaskIds;
+        for (const Subtask& subtask : constraints_) {
+            if (subtask.id() != -1) {
+                definedSubtaskIds.insert(subtask.id());
+            }
+        }
+        return definedSubtaskIds;
+    }
 };
 
 class ConstraintSuiteBuilder {

include/tcframe/spec/core/BaseTestSpec.hpp

@@ -1,6 +1,5 @@
 #pragma once
 
-#include <set>
 #include <vector>
 
 #include "CommonConfig.hpp"
@@ -11,7 +10,6 @@
 #include "tcframe/spec.hpp"
 #include "tcframe/util.hpp"
 
-using std::set;
 using std::vector;
 
 namespace tcframe {
@@ -76,8 +74,9 @@ class BaseTestSpec : public TProblemSpec, protected TestSuiteBuilder {
 public:
     virtual ~BaseTestSpec() {}
 
-    TestSuite buildTestSuite(const string& slug) {
+    TestSuite buildTestSuite(const string& slug, const set<int>& definedSubtaskIds) {
         TestSuiteBuilder::setSlug(slug);
+        TestSuiteBuilder::setDefinedSubtaskIds(definedSubtaskIds);
         TestSuiteBuilder::setBeforeClosure([this] {
             BeforeTestCase();
         });
@@ -116,7 +115,7 @@ class BaseTestSpec : public TProblemSpec, protected TestSuiteBuilder {
         GradingConfig gradingConfig = TProblemSpec::buildGradingConfig();
         MultipleTestCasesConfig multipleTestCasesConfig = TProblemSpec::buildMultipleTestCasesConfig();
         ConstraintSuite constraintSuite = TProblemSpec::buildConstraintSuite();
-        TestSuite testSuite = buildTestSuite(slug);
+        TestSuite testSuite = buildTestSuite(slug, constraintSuite.getDefinedSubtaskIds());
         return Spec(seedSetter,
                     ioFormat,
                     styleConfig,

include/tcframe/spec/testcase/TestSuite.hpp

@@ -1,7 +1,9 @@
 #pragma once
 
+#include <algorithm>
 #include <functional>
 #include <set>
+#include <stdexcept>
 #include <string>
 #include <tuple>
 #include <vector>
@@ -11,9 +13,13 @@
 #include "TestCase.hpp"
 #include "TestCaseIdCreator.hpp"
 #include "TestGroup.hpp"
+#include "tcframe/util.hpp"
 
+using std::inserter;
 using std::function;
+using std::runtime_error;
 using std::set;
+using std::set_difference;
 using std::string;
 using std::tie;
 using std::vector;
@@ -44,6 +50,7 @@ struct TestSuite {
 class TestSuiteBuilder {
 private:
     string slug_;
+    set<int> definedSubtaskIds_;
     function<void()> beforeClosure_;
     function<void()> afterClosure_;
 
@@ -79,6 +86,11 @@ class TestSuiteBuilder {
         return *this;
     }
 
+    TestSuiteBuilder& setDefinedSubtaskIds(set<int> definedSubtaskIds) {
+        definedSubtaskIds_ = definedSubtaskIds;
+        return *this;
+    }
+
     TestSuiteBuilder& setBeforeClosure(function<void()> beforeClosure) {
         beforeClosure_ = beforeClosure;
         return *this;
@@ -120,6 +132,20 @@ class TestSuiteBuilder {
     }
 
     TestSuiteBuilder& Subtasks(set<int> subtaskIds) {
+        set<int> undefinedSubtaskIds;
+        set_difference(
+                subtaskIds.begin(),
+                subtaskIds.end(),
+                definedSubtaskIds_.begin(),
+                definedSubtaskIds_.end(),
+                inserter(undefinedSubtaskIds, undefinedSubtaskIds.begin()));
+
+        if (!undefinedSubtaskIds.empty()) {
+            throw runtime_error(
+                    "The following subtasks are referenced but not defined: " +
+                    StringUtils::setToString(undefinedSubtaskIds));
+        }
+
         *curSubtaskIds_ = subtaskIds;
         return *this;
     }

include/tcframe/util/StringUtils.hpp

@@ -1,6 +1,7 @@
 #pragma once
 
 #include <istream>
+#include <set>
 #include <sstream>
 #include <streambuf>
 #include <string>
@@ -12,6 +13,7 @@ using std::istream;
 using std::istreambuf_iterator;
 using std::istringstream;
 using std::ostringstream;
+using std::set;
 using std::string;
 using std::vector;
 
@@ -32,6 +34,22 @@ class StringUtils {
         return string(istreambuf_iterator<char>(*in), istreambuf_iterator<char>());
     }
 
+    template<typename T>
+    static string setToString(const set<T>& set) {
+        ostringstream out;
+        out << "{";
+        bool any = false;
+        for (const T& val : set) {
+            if (any) {
+                out << ", ";
+            }
+            any = true;
+            out << val;
+        }
+        out << "}";
+        return out.str();
+    }
+
     template<typename T>
     static optional<T> toNumber(const string& s) {
         istringstream in(s);

test/unit/tcframe/spec/core/BaseTestSpecTests.cpp

@@ -8,6 +8,7 @@ using ::testing::ElementsAre;
 using ::testing::Eq;
 using ::testing::Property;
 using ::testing::SizeIs;
+using ::testing::StrEq;
 using ::testing::Test;
 
 namespace tcframe {
@@ -19,9 +20,16 @@ class BaseTestSpecTests : public Test {
         int A, B;
         vector<int> V;
 
+    protected:
         void InputFormat() {}
     };
 
+    class ProblemSpecWithSubtasks : public ProblemSpec {
+    protected:
+        void Subtask1() {}
+        void Subtask2() {}
+    };
+
     class TestSpecWithLifecycle : public BaseTestSpec<ProblemSpec> {
     protected:
         void BeforeTestCase() {
@@ -55,7 +63,7 @@ class BaseTestSpecTests : public Test {
         }
     };
 
-    class TestSpecWithTestGroups : public BaseTestSpec<ProblemSpec> {
+    class TestSpecWithTestGroups : public BaseTestSpec<ProblemSpecWithSubtasks> {
     protected:
         void SampleTestCase1() {
             Subtasks({1, 2});
@@ -92,6 +100,13 @@ class BaseTestSpecTests : public Test {
         }
     };
 
+    class TestSpecWithInvalidTestGroups : public BaseTestSpec<ProblemSpecWithSubtasks> {
+    protected:
+        void TestGroup1() {
+            Subtasks({1, 3, 5});
+        }
+    };
+
     // Testing that rnd is available in TestSpec
     class TestSpecWithRandom : public BaseTestSpec<ProblemSpec> {
     protected:
@@ -103,10 +118,11 @@ class BaseTestSpecTests : public Test {
     TestSpecWithLifecycle testSpecWithLifecycle;
     TestSpecWithTestCases testSpecWithTestCases;
     TestSpecWithTestGroups testSpecWithTestGroups;
+    TestSpecWithInvalidTestGroups testSpecWithInvalidTestGroups;
 };
 
 TEST_F(BaseTestSpecTests, Lifecycle) {
-    TestSuite testSuite = testSpecWithLifecycle.buildTestSuite("foo");
+    TestSuite testSuite = testSpecWithLifecycle.buildTestSuite("foo", {});
     OfficialTestCaseData* tc1 = (OfficialTestCaseData*) testSuite.testGroups()[1].testCases()[0].data();
     OfficialTestCaseData* tc2 = (OfficialTestCaseData*) testSuite.testGroups()[1].testCases()[1].data();
 
@@ -118,18 +134,26 @@ TEST_F(BaseTestSpecTests, Lifecycle) {
 }
 
 TEST_F(BaseTestSpecTests, TestSuite) {
-    TestSuite testSuite = testSpecWithTestCases.buildTestSuite("foo");
+    TestSuite testSuite = testSpecWithTestCases.buildTestSuite("foo", {});
     EXPECT_THAT(testSuite.testGroups(), ElementsAre(
             AllOf(Property(&TestGroup::id, 0), Property(&TestGroup::testCases, SizeIs(2))),
             AllOf(Property(&TestGroup::id, -1), Property(&TestGroup::testCases, SizeIs(2)))));
 }
 
 TEST_F(BaseTestSpecTests, TestSuite_WithGroups) {
-    TestSuite testSuite = testSpecWithTestGroups.buildTestSuite("foo");
+    TestSuite testSuite = testSpecWithTestGroups.buildTestSuite("foo", {1, 2});
     EXPECT_THAT(testSuite.testGroups(), ElementsAre(
             AllOf(Property(&TestGroup::id, 0), Property(&TestGroup::testCases, SizeIs(2))),
             AllOf(Property(&TestGroup::id, 1), Property(&TestGroup::testCases, SizeIs(3))),
             AllOf(Property(&TestGroup::id, 2), Property(&TestGroup::testCases, SizeIs(2)))));
 }
 
+TEST_F(BaseTestSpecTests, Spec_Invalid_UndefinedSubtaskIds) {
+    try {
+        testSpecWithInvalidTestGroups.buildSpec("foo");
+    } catch (runtime_error& e) {
+        EXPECT_THAT(e.what(), StrEq("The following subtasks are referenced but not defined: {3, 5}"));
+    }
+}
+
 }

test/unit/tcframe/spec/testcase/TestSuiteBuilderTests.cpp

@@ -12,9 +12,9 @@ class TestSuiteBuilderTests : public Test {
     static int N;
 
 protected:
-    TestSuiteBuilder builder = TestSuiteBuilder().setSlug("foo");
-    TestSuiteBuilder builder1 = TestSuiteBuilder().setSlug("foo");
-    TestSuiteBuilder builder2 = TestSuiteBuilder().setSlug("foo");
+    TestSuiteBuilder builder = TestSuiteBuilder().setSlug("foo").setDefinedSubtaskIds({1, 2, 3});
+    TestSuiteBuilder builder1 = TestSuiteBuilder().setSlug("foo").setDefinedSubtaskIds({1, 2, 3});
+    TestSuiteBuilder builder2 = TestSuiteBuilder().setSlug("foo").setDefinedSubtaskIds({1, 2, 3});
 };
 
 int TestSuiteBuilderTests::N;

test/unit/tcframe/util/StringUtilsTests.cpp

@@ -22,6 +22,12 @@ TEST_F(StringUtilsTests, StreamToString) {
     EXPECT_THAT(StringUtils::streamToString(new istringstream(" hello, world! ")), Eq(" hello, world! "));
 }
 
+TEST_F(StringUtilsTests, SetToString) {
+    EXPECT_THAT(StringUtils::setToString(set<int>{}), Eq("{}"));
+    EXPECT_THAT(StringUtils::setToString(set<int>{1}), Eq("{1}"));
+    EXPECT_THAT(StringUtils::setToString(set<int>{1, 2, 3}), Eq("{1, 2, 3}"));
+}
+
 TEST_F(StringUtilsTests, ToNumber) {
     EXPECT_THAT(StringUtils::toNumber<int>("42"), Eq(optional<int>(42)));
     EXPECT_THAT(StringUtils::toNumber<int>("-42"), Eq(optional<int>(-42)));