Merge topic 'ctest-resource-fixes'

a033bafbe0 Help: Clarify how tests are run if no resource spec file is specified
a64ba0235f CTest: Clarify that resource requirements can be split
f9f294f5fa CTest: Add version field to resource spec file

Acked-by: Kitware Robot <[email protected]>
Merge-request: !4080

Help/manual/ctest.1.rst

@@ -1348,6 +1348,23 @@ the :ref:`environment variables <ctest-resource-environment-variables>` to
 determine which resources have been allocated to each group.  For example,
 each group may correspond to a process the test will spawn when executed.
 
+Note that even if a test specifies a ``RESOURCE_GROUPS`` property, it is still
+possible for that to test to run without any resource allocation (and without
+the corresponding
+:ref:`environment variables <ctest-resource-environment-variables>`)
+if the user does not pass a resource specification file. Passing this file,
+either through the ``--resource-spec-file`` command-line argument or the
+``RESOURCE_SPEC_FILE`` argument to :command:`ctest_test`, is what activates the
+resource allocation feature. Tests should check the
+``CTEST_RESOURCE_GROUP_COUNT`` environment variable to find out whether or not
+resource allocation is activated. This variable will always (and only) be
+defined if resource allocation is activated. If resource allocation is not
+activated, then the ``CTEST_RESOURCE_GROUP_COUNT`` variable will not exist,
+even if it exists for the parent ``ctest`` process. If a test absolutely must
+have resource allocation, then it can return a failing exit code or use the
+:prop_test:`SKIP_RETURN_CODE` or :prop_test:`SKIP_REGULAR_EXPRESSION`
+properties to indicate a skipped test.
+
 .. _`ctest-resource-specification-file`:
 
 Resource Specification File
@@ -1362,6 +1379,10 @@ the following resource specification file:
 .. code-block:: json
 
   {
+    "version": {
+      "major": 1,
+      "minor": 0
+    },
     "local": [
       {
         "gpus": [
@@ -1393,6 +1414,11 @@ the following resource specification file:
 
 The members are:
 
+``version``
+  An object containing a ``major`` integer field and a ``minor`` integer field.
+  Currently, the only supported version is major ``1``, minor ``0``. Any other
+  value is an error.
+
 ``local``
   A JSON array of resource sets present on the system.  Currently, this array
   is restricted to being of size 1.

Help/prop_test/RESOURCE_GROUPS.rst

@@ -38,6 +38,22 @@ This single group requires 4 slots from a single GPU and 2 slots from a
 single cryptography chip. In total, 3 resource groups are specified for this
 test, each with its own unique requirements.
 
+Note that the number of slots following the resource type specifies slots from
+a *single* instance of the resource. If the resource group can tolerate
+receiving slots from different instances of the same resource, it can indicate
+this by splitting the specification into multiple requirements of one slot. For
+example:
+
+.. code-block:: cmake
+
+  add_test(NAME MyTest COMMAND MyExe)
+  set_property(TEST MyTest PROPERTY RESOURCE_GROUPS
+    "gpus:1,gpus:1,gpus:1,gpus:1")
+
+In this case, the single resource group indicates that it needs four GPU slots,
+all of which may come from separate GPUs (though they don't have to; CTest may
+still assign slots from the same GPU.)
+
 When CTest sets the :ref:`environment variables
 <ctest-resource-environment-variables>` for a test, it assigns a group number
 based on the group description, starting at 0 on the left and the number of

Source/CTest/cmCTestResourceSpec.cxx

@@ -33,6 +33,32 @@ bool cmCTestResourceSpec::ReadFromJSONFile(const std::string& filename)
     return false;
   }
 
+  int majorVersion = 1;
+  int minorVersion = 0;
+  if (root.isMember("version")) {
+    auto const& version = root["version"];
+    if (version.isObject()) {
+      if (!version.isMember("major") || !version.isMember("minor")) {
+        return false;
+      }
+      auto const& major = version["major"];
+      auto const& minor = version["minor"];
+      if (!major.isInt() || !minor.isInt()) {
+        return false;
+      }
+      majorVersion = major.asInt();
+      minorVersion = minor.asInt();
+    } else {
+      return false;
+    }
+  } else {
+    return false;
+  }
+
+  if (majorVersion != 1 || minorVersion != 0) {
+    return false;
+  }
+
   auto const& local = root["local"];
   if (!local.isArray()) {
     return false;

Tests/CMakeLib/testCTestResourceGroups.cxx

@@ -74,6 +74,9 @@ static const std::vector<ExpectedParseResult> expectedResults{
   { "1,threads:1,", true, {
     { { "threads", 1, 1 } },
   } },
+  { "threads:1,threads:1", true, {
+    { { "threads", 1, 1 }, { "threads", 1, 1 } },
+  } },
   { "threads:1;;threads:2", true, {
     { { "threads", 1, 1 } },
     { { "threads", 2, 1 } },

Tests/CMakeLib/testCTestResourceSpec.cxx

@@ -39,6 +39,23 @@ static const std::vector<ExpectedSpec> expectedResourceSpecs = {
   {"spec16.json", true, {{{}}}},
   {"spec17.json", false, {{{}}}},
   {"spec18.json", false, {{{}}}},
+  {"spec19.json", false, {{{}}}},
+  {"spec20.json", true, {{{}}}},
+  {"spec21.json", false, {{{}}}},
+  {"spec22.json", false, {{{}}}},
+  {"spec23.json", false, {{{}}}},
+  {"spec24.json", false, {{{}}}},
+  {"spec25.json", false, {{{}}}},
+  {"spec26.json", false, {{{}}}},
+  {"spec27.json", false, {{{}}}},
+  {"spec28.json", false, {{{}}}},
+  {"spec29.json", false, {{{}}}},
+  {"spec30.json", false, {{{}}}},
+  {"spec31.json", false, {{{}}}},
+  {"spec32.json", false, {{{}}}},
+  {"spec33.json", false, {{{}}}},
+  {"spec34.json", false, {{{}}}},
+  {"spec35.json", false, {{{}}}},
   {"noexist.json", false, {{{}}}},
   /* clang-format on */
 };

Tests/CMakeLib/testCTestResourceSpec_data/spec1.json

@@ -1,4 +1,8 @@
 {
+  "version": {
+    "major": 1,
+    "minor": 0
+  },
   "local": [
     {
       "gpus": [

Tests/CMakeLib/testCTestResourceSpec_data/spec10.json

@@ -1,4 +1,8 @@
 {
+  "version": {
+    "major": 1,
+    "minor": 0
+  },
   "local": [
     {
       "gpus": [

Tests/CMakeLib/testCTestResourceSpec_data/spec11.json

@@ -1,4 +1,8 @@
 {
+  "version": {
+    "major": 1,
+    "minor": 0
+  },
   "local": [
     {
       "gpus": [

Tests/CMakeLib/testCTestResourceSpec_data/spec14.json

@@ -1,4 +1,8 @@
 {
+  "version": {
+    "major": 1,
+    "minor": 0
+  },
   "local": [
     {
       "0": [

Tests/CMakeLib/testCTestResourceSpec_data/spec15.json

@@ -1,4 +1,8 @@
 {
+  "version": {
+    "major": 1,
+    "minor": 0
+  },
   "local": [
     {
       "-": [

Tests/CMakeLib/testCTestResourceSpec_data/spec16.json

@@ -1,4 +1,8 @@
 {
+  "version": {
+    "major": 1,
+    "minor": 0
+  },
   "local": [
     {
       "A": [

Tests/CMakeLib/testCTestResourceSpec_data/spec17.json

@@ -1,4 +1,8 @@
 {
+  "version": {
+    "major": 1,
+    "minor": 0
+  },
   "local": [
     {
       "gpus": [

Tests/CMakeLib/testCTestResourceSpec_data/spec18.json

@@ -1,4 +1,8 @@
 {
+  "version": {
+    "major": 1,
+    "minor": 0
+  },
   "local": [
     {
       "gpus": [

Tests/CMakeLib/testCTestResourceSpec_data/spec19.json

@@ -0,0 +1,5 @@
+{
+  "version": 1,
+  "local": [
+  ]
+}

Tests/CMakeLib/testCTestResourceSpec_data/spec2.json

@@ -1,4 +1,8 @@
 {
+  "version": {
+    "major": 1,
+    "minor": 0
+  },
   "local": [
   ]
 }

Tests/CMakeLib/testCTestResourceSpec_data/spec20.json

@@ -0,0 +1,8 @@
+{
+  "version": {
+    "major": 1,
+    "minor": 0
+  },
+  "local": [
+  ]
+}

Tests/CMakeLib/testCTestResourceSpec_data/spec21.json

@@ -0,0 +1,5 @@
+{
+  "version": [1, 0],
+  "local": [
+  ]
+}

Tests/CMakeLib/testCTestResourceSpec_data/spec22.json

@@ -0,0 +1,5 @@
+{
+  "version": 2,
+  "local": [
+  ]
+}

Tests/CMakeLib/testCTestResourceSpec_data/spec23.json

@@ -0,0 +1,7 @@
+{
+  "version": {
+    "major": 1
+  },
+  "local": [
+  ]
+}

Tests/CMakeLib/testCTestResourceSpec_data/spec24.json

@@ -0,0 +1,7 @@
+{
+  "version": {
+    "minor": 0
+  },
+  "local": [
+  ]
+}

Tests/CMakeLib/testCTestResourceSpec_data/spec25.json

@@ -0,0 +1,8 @@
+{
+  "version": {
+    "major": 1,
+    "minor": 1
+  },
+  "local": [
+  ]
+}

Tests/CMakeLib/testCTestResourceSpec_data/spec26.json

@@ -0,0 +1,8 @@
+{
+  "version": {
+    "major": 2,
+    "minor": 0
+  },
+  "local": [
+  ]
+}

Tests/CMakeLib/testCTestResourceSpec_data/spec27.json

@@ -0,0 +1,8 @@
+{
+  "version": {
+    "major": "1",
+    "minor": 0
+  },
+  "local": [
+  ]
+}

Tests/CMakeLib/testCTestResourceSpec_data/spec28.json

@@ -0,0 +1,8 @@
+{
+  "version": {
+    "major": 1,
+    "minor": "0"
+  },
+  "local": [
+  ]
+}

Tests/CMakeLib/testCTestResourceSpec_data/spec29.json

@@ -0,0 +1,5 @@
+{
+  "version": [1, 0, 0],
+  "local": [
+  ]
+}

Tests/CMakeLib/testCTestResourceSpec_data/spec3.json

@@ -1,4 +1,8 @@
 {
+  "version": {
+    "major": 1,
+    "minor": 0
+  },
   "local": [
     {
     },

Tests/CMakeLib/testCTestResourceSpec_data/spec30.json

@@ -0,0 +1,5 @@
+{
+  "version": [1],
+  "local": [
+  ]
+}

Tests/CMakeLib/testCTestResourceSpec_data/spec31.json

@@ -0,0 +1,5 @@
+{
+  "version": [1, 1],
+  "local": [
+  ]
+}

Tests/CMakeLib/testCTestResourceSpec_data/spec32.json

@@ -0,0 +1,5 @@
+{
+  "version": [2, 0],
+  "local": [
+  ]
+}

Tests/CMakeLib/testCTestResourceSpec_data/spec33.json

@@ -0,0 +1,5 @@
+{
+  "version": ["1", 0],
+  "local": [
+  ]
+}

Tests/CMakeLib/testCTestResourceSpec_data/spec34.json

@@ -0,0 +1,5 @@
+{
+  "version": [1, "0"],
+  "local": [
+  ]
+}

Tests/CMakeLib/testCTestResourceSpec_data/spec35.json

@@ -0,0 +1,5 @@
+{
+  "version": "1",
+  "local": [
+  ]
+}

Tests/CMakeLib/testCTestResourceSpec_data/spec4.json

@@ -1,4 +1,8 @@
 {
+  "version": {
+    "major": 1,
+    "minor": 0
+  },
   "local": {
   }
 }

Tests/CMakeLib/testCTestResourceSpec_data/spec5.json

@@ -1,2 +1,6 @@
 {
+  "version": {
+    "major": 1,
+    "minor": 0
+  }
 }

Tests/CMakeLib/testCTestResourceSpec_data/spec6.json

@@ -1,4 +1,8 @@
 {
+  "version": {
+    "major": 1,
+    "minor": 0
+  },
   "local": [
     []
   ]

Tests/CMakeLib/testCTestResourceSpec_data/spec7.json

@@ -1,4 +1,8 @@
 {
+  "version": {
+    "major": 1,
+    "minor": 0
+  },
   "local": [
     {
       "gpus": {

Tests/CMakeLib/testCTestResourceSpec_data/spec8.json

@@ -1,4 +1,8 @@
 {
+  "version": {
+    "major": 1,
+    "minor": 0
+  },
   "local": [
     {
       "gpus": [

Tests/CMakeLib/testCTestResourceSpec_data/spec9.json

@@ -1,4 +1,8 @@
 {
+  "version": {
+    "major": 1,
+    "minor": 0
+  },
   "local": [
     {
       "gpus": [

Tests/RunCMake/CTestResourceAllocation/RunCMakeTest.cmake

@@ -160,6 +160,8 @@ run_ctest_resource(checkfree1 2 0)
 run_ctest_resource(checkfree2 1 0)
 run_ctest_resource(notenough1 1 0)
 run_ctest_resource(notenough2 1 0)
+run_ctest_resource(notenough3 1 0)
+run_ctest_resource(combine 1 0)
 run_ctest_resource(ensure_parallel 2 0)
 
 set(ENV{CTEST_RESOURCE_GROUP_COUNT} 2)

Tests/RunCMake/CTestResourceAllocation/combine.cmake

@@ -0,0 +1,5 @@
+setup_resource_tests()
+
+add_resource_test(Test1 0 "widgets:8,widgets:4")
+
+cleanup_resource_tests()

Tests/RunCMake/CTestResourceAllocation/notenough1.cmake

@@ -1,5 +1,5 @@
 setup_resource_tests()
 
-add_resource_test(Test1 1 "fluxcapacitors:200")
+add_resource_test(Test1 0 "fluxcapacitors:200")
 
 cleanup_resource_tests()

Tests/RunCMake/CTestResourceAllocation/notenough2.cmake

@@ -1,5 +1,5 @@
 setup_resource_tests()
 
-add_resource_test(Test1 1 "terminators:2")
+add_resource_test(Test1 0 "terminators:2")
 
 cleanup_resource_tests()

Tests/RunCMake/CTestResourceAllocation/notenough3-ctest-s-res-check.cmake

@@ -0,0 +1,3 @@
+if(EXISTS "${RunCMake_TEST_BINARY_DIR}/ctresalloc.log")
+  set(RunCMake_TEST_FAILED "ctresalloc.log should not exist")
+endif()

Tests/RunCMake/CTestResourceAllocation/notenough3-ctest-s-res-result.txt

@@ -0,0 +1 @@
+(-1|255)

Tests/RunCMake/CTestResourceAllocation/notenough3-ctest-s-res-stderr.txt

@@ -0,0 +1,4 @@
+^Insufficient resources
+CMake Error at [^
+]*/Tests/RunCMake/CTestResourceAllocation/notenough3-ctest-s-res/test\.cmake:[0-9]+ \(message\):
+  Tests did not pass$

Tests/RunCMake/CTestResourceAllocation/notenough3.cmake

@@ -0,0 +1,5 @@
+setup_resource_tests()
+
+add_resource_test(Test1 0 "widgets:12")
+
+cleanup_resource_tests()

Tests/RunCMake/CTestResourceAllocation/resspec.json

@@ -1,4 +1,8 @@
 {
+  "version": {
+    "major": 1,
+    "minor": 0
+  },
   "local": [
     {
       "widgets": [