Make sure to report an error if process.options.numberOfThreads has w…

…rong type
cms-sw · May 10, 2023 · 46315ee · 46315ee
1 parent d327c97
commit 46315ee
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 4 deletions.
diff --git a/FWCore/Framework/test/BuildFile.xml b/FWCore/Framework/test/BuildFile.xml
@@ -419,3 +419,8 @@
 <test name="testFWCoreFrameworkConditionalTask_testView_testAlias_aliasWithStar" command="cmsRun ${LOCALTOP}/src/FWCore/Framework/test/test_conditionaltasks_cfg.py -- --testView --testAlias --aliasWithStar"/>
 <test name="testFWCoreFrameworkConditionalTask_testConsumesMany" command="cmsRun ${LOCALTOP}/src/FWCore/Framework/test/test_conditionaltasks_cfg.py -- --testConsumesMany"/>
 <test name="testFWCoreFrameworkModuleSynchLumiBoundary" command="run_module_synch_lumiboundary.sh"/>
+
+<test name="testFWCoreFrameworkOptionsNumberOfThreadsType" command="cmsRun ${LOCALTOP}/src/FWCore/Framework/test/test_wrongOptionsType_cfg.py -- --name=numberOfThreads --value='cms.untracked.uint32(1)'"/>
+<test name="testFWCoreFrameworkWrongOptionsNumberOfThreadsType" command="run_wrongOptionsType.sh numberOfThreads 'cms.untracked.int32(1)'"/>
+<test name="testFWCoreFrameworkWrongOptionsNumberOfStreamsType" command="run_wrongOptionsType.sh numberOfStreams 'cms.untracked.int32(1)'"/>
+<test name="testFWCoreFrameworkWrongOptionssizeOfStackForThreadsInKBType" command="run_wrongOptionsType.sh sizeOfStackForThreadsInKB 'cms.untracked.int32(1024)'"/>
diff --git a/FWCore/Framework/test/run_wrongOptionsType.sh b/FWCore/Framework/test/run_wrongOptionsType.sh
@@ -0,0 +1,10 @@
+#!/bin/bash
+
+NAME=$1
+VALUE=$2
+
+function die { echo $1: status $2 ; echo === Log file === ; cat ${3:-/dev/null} ; echo === End log file === ; exit $2; }
+
+cmsRun ${SCRAM_TEST_PATH}/test_wrongOptionsType_cfg.py -- --name=${NAME} --value="$VALUE" > ${NAME}.log 2>&1 && die "cmsRun for ${NAME} succeeded, should have failed" 1 ${NAME}.log
+grep -E "(The type in the configuration is incorrect)|(ValueError type of .* is expected to be .* but declared as)" ${NAME}.log > /dev/null || die "cmsRun for ${NAME} failed for other reason than incorrect configuration type" $? ${NAME}.log
+
diff --git a/FWCore/Framework/test/test_wrongOptionsType_cfg.py b/FWCore/Framework/test/test_wrongOptionsType_cfg.py
@@ -0,0 +1,21 @@
+import FWCore.ParameterSet.Config as cms
+
+import argparse
+import sys
+
+parser = argparse.ArgumentParser(prog=sys.argv[0], description='Test wrong process.options parameter types')
+
+parser.add_argument("--name", help="Name of parameter", type=str)
+parser.add_argument("--value", help="Value of the parameter", type=str)
+
+argv = sys.argv[:]
+if '--' in argv:
+    argv.remove("--")
+args, unknown = parser.parse_known_args(argv)
+
+process = cms.Process("TEST")
+process.source = cms.Source("EmptySource")
+
+process.maxEvents.input = 2
+
+setattr(process.options, args.name, eval(str(args.value)))
diff --git a/FWCore/ParameterSet/src/ThreadsInfo.cc b/FWCore/ParameterSet/src/ThreadsInfo.cc
@@ -12,12 +12,15 @@ namespace edm {
     // default values
     ThreadsInfo threadsInfo;
 
-    if (pset.existsAs<edm::ParameterSet>("options", false)) {
+    // Note: it is important to not check the type or trackedness in
+    // exists() call to ensure that the getUntrackedParameter() calls
+    // will fail if the parameters have an incorrect type
+    if (pset.exists("options")) {
       auto const& ops = pset.getUntrackedParameterSet("options");
-      if (ops.existsAs<unsigned int>("numberOfThreads", false)) {
+      if (ops.exists("numberOfThreads")) {
         threadsInfo.nThreads_ = ops.getUntrackedParameter<unsigned int>("numberOfThreads");
       }
-      if (ops.existsAs<unsigned int>("sizeOfStackForThreadsInKB", false)) {
+      if (ops.exists("sizeOfStackForThreadsInKB")) {
         threadsInfo.stackSize_ = ops.getUntrackedParameter<unsigned int>("sizeOfStackForThreadsInKB");
       }
     }
@@ -26,7 +29,7 @@ namespace edm {
 
   void setThreadOptions(ThreadsInfo const& threadsInfo, edm::ParameterSet& pset) {
     edm::ParameterSet newOp;
-    if (pset.existsAs<edm::ParameterSet>("options", false)) {
+    if (pset.exists("options")) {
       newOp = pset.getUntrackedParameterSet("options");
     }
     newOp.addUntrackedParameter<unsigned int>("numberOfThreads", threadsInfo.nThreads_);