From 9f295b4526f91db1cf3748b7ae2fe3f91cb981f5 Mon Sep 17 00:00:00 2001 From: lobis Date: Mon, 9 May 2022 18:41:23 +0200 Subject: [PATCH 01/16] TRestGDMLParser.cxx - better debugging message --- source/framework/core/src/TRestGDMLParser.cxx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/framework/core/src/TRestGDMLParser.cxx b/source/framework/core/src/TRestGDMLParser.cxx index ef35686f4..49c6541f5 100644 --- a/source/framework/core/src/TRestGDMLParser.cxx +++ b/source/framework/core/src/TRestGDMLParser.cxx @@ -60,15 +60,15 @@ void TRestGDMLParser::Load(string file) { ReplaceAttributeWithKeyWord("log("); ReplaceAttributeWithKeyWord("exp("); - cout << "GDML: creating temporary file" << endl; ofstream outf; string fname = TRestTools::SeparatePathAndName(file).second; // we have to use a unique identifier on the file to prevent collision when launching multiple jobs outfilename = outPath + "PID" + std::to_string(getpid()) + "_" + fname; + cout << "GDML: creating temporary file at: \"" << outfilename << "\"" << endl; + outf.open(outfilename, ios::trunc); outf << filestr << endl; outf.close(); - // getchar(); } else { ferr << "Filename : " << file << endl; From f0ebe598af553a38dc2b4617630e0cea0be53402 Mon Sep 17 00:00:00 2001 From: lobis Date: Mon, 9 May 2022 18:54:43 +0200 Subject: [PATCH 02/16] TRestGDMLParser - modernized class, normalized member names, increased class version (no functional changes) --- source/framework/core/inc/TRestGDMLParser.h | 32 ++--- source/framework/core/src/TRestGDMLParser.cxx | 114 +++++++++--------- 2 files changed, 71 insertions(+), 75 deletions(-) diff --git a/source/framework/core/inc/TRestGDMLParser.h b/source/framework/core/inc/TRestGDMLParser.h index a09e8018e..52fc92b07 100644 --- a/source/framework/core/inc/TRestGDMLParser.h +++ b/source/framework/core/inc/TRestGDMLParser.h @@ -13,30 +13,30 @@ class TRestGDMLParser : public TRestMetadata { private: - TGeoManager* fGeo; + TGeoManager* fGeoManager; public: TRestGDMLParser() {} ~TRestGDMLParser() {} - std::string filestr = ""; - std::string path = ""; - std::string outPath = REST_USER_PATH + "/gdml/"; - std::string outfilename = ""; - std::string gdmlVersion = "0.0"; - std::map entityVersion; + std::string fFileString; + std::string fPath; + std::string fOutputGdmlDirectory = REST_USER_PATH + "/gdml/"; + std::string fOutputGdmlFilename; + std::string fGdmlVersion = "0.0"; + std::map fEntityVersionMap; - std::string GetEntityVersion(std::string name); + std::string GetEntityVersion(const std::string& name) const; - std::string GetGDMLVersion(); + inline std::string GetGDMLVersion() const { return fGdmlVersion; } - std::string GetOutputGDMLFile(); + inline std::string GetOutputGDMLFile() const { return fOutputGdmlFilename; } - void Load(std::string file); + void Load(const std::string& filename); - TGeoManager* GetGeoManager(std::string gdmlFile) { - Load(gdmlFile); - fGeo = TGeoManager::Import(GetOutputGDMLFile().c_str()); - return fGeo; + inline TGeoManager* GetGeoManager(const std::string& gdmlFilename) { + Load(gdmlFilename); + fGeoManager = TGeoManager::Import(GetOutputGDMLFile().c_str()); + return fGeoManager; } TGeoManager* CreateGeoM(); @@ -49,7 +49,7 @@ class TRestGDMLParser : public TRestMetadata { void ReplaceAttributeWithKeyWord(std::string keyword); - ClassDef(TRestGDMLParser, 1); + ClassDef(TRestGDMLParser, 2); }; #endif diff --git a/source/framework/core/src/TRestGDMLParser.cxx b/source/framework/core/src/TRestGDMLParser.cxx index 49c6541f5..9430b6b74 100644 --- a/source/framework/core/src/TRestGDMLParser.cxx +++ b/source/framework/core/src/TRestGDMLParser.cxx @@ -2,44 +2,39 @@ using namespace std; -string TRestGDMLParser::GetEntityVersion(string name) { - for (auto& i : entityVersion) { - if (i.first == name) { - return i.second; - break; +string TRestGDMLParser::GetEntityVersion(const string& name) const { + for (auto& [entityName, entityVersion] : fEntityVersionMap) { + if (entityName == name) { + return entityVersion; } } return "0.0"; } -string TRestGDMLParser::GetGDMLVersion() { return gdmlVersion; } +void TRestGDMLParser::Load(const string& filename) { + const auto filenameAbsolute = TRestTools::ToAbsoluteName(filename); + if (TRestTools::fileExists(filenameAbsolute)) { + fConfigFileName = filenameAbsolute; + fPath = TRestTools::SeparatePathAndName(filenameAbsolute).first; -string TRestGDMLParser::GetOutputGDMLFile() { return outfilename; } - -void TRestGDMLParser::Load(string file) { - file = TRestTools::ToAbsoluteName(file); - if (TRestTools::fileExists(file)) { - fConfigFileName = file; - path = TRestTools::SeparatePathAndName(file).first; - - std::ifstream t(file); + std::ifstream t(filenameAbsolute); std::string str((std::istreambuf_iterator(t)), std::istreambuf_iterator()); - filestr = str; + fFileString = str; t.close(); - int pp = filestr.find("##VERSION", 0); + int pp = fFileString.find("##VERSION", 0); if (pp != string::npos) { - int pp2 = filestr.find("##", pp + 4); - if (pp2 != string::npos) gdmlVersion = filestr.substr(pp + 9, pp2 - pp - 9); - gdmlVersion = ReplaceMathematicalExpressions(ReplaceConstants(ReplaceVariables(gdmlVersion))); + int pp2 = fFileString.find("##", pp + 4); + if (pp2 != string::npos) fGdmlVersion = fFileString.substr(pp + 9, pp2 - pp - 9); + fGdmlVersion = ReplaceMathematicalExpressions(ReplaceConstants(ReplaceVariables(fGdmlVersion))); } - filestr = ReplaceConstants(ReplaceVariables(filestr)); + fFileString = ReplaceConstants(ReplaceVariables(fFileString)); cout << "GDML: initializating variables" << endl; - int pos = filestr.find("Import(outfilename.c_str()); + geo2->Import(fOutputGdmlFilename.c_str()); chdir(originDirectory); return geo2; } return nullptr; } -void TRestGDMLParser::PrintContent() { cout << filestr << endl; } +void TRestGDMLParser::PrintContent() { cout << fFileString << endl; } void TRestGDMLParser::ReplaceEntity() { int pos = 0; - while ((pos = filestr.find("(t)), std::istreambuf_iterator()); @@ -128,24 +124,24 @@ void TRestGDMLParser::ReplaceEntity() { str = ReplaceConstants(ReplaceVariables(str)); - entityVersion[entityname] = ""; + fEntityVersionMap[entityName] = ""; int pp = str.find("##VERSION", 0); if (pp != string::npos) { int pp2 = str.find("##", pp + 4); if (pp2 != string::npos) { - entityVersion[entityname] = str.substr(pp + 9, pp2 - pp - 9); + fEntityVersionMap[entityName] = str.substr(pp + 9, pp2 - pp - 9); } } // cout << "replacing entity..." << endl; - filestr.replace(pos5, entityname.length() + 2, str); + fFileString.replace(pos5, entityName.length() + 2, str); } else { - cout << "GDML ERROR! No matching reference file for entity: \"" << entityname << "\"" << endl; + cout << "GDML ERROR! No matching reference file for entity: \"" << entityName << "\"" << endl; cout << "file name: \"" << entityfile << "\"" << endl; exit(0); } } else { - cout << "GDML: Warning! redundant entity: \"" << entityname << "\"" << endl; + cout << "GDML: Warning! redundant entity: \"" << entityName << "\"" << endl; } pos++; } @@ -153,22 +149,22 @@ void TRestGDMLParser::ReplaceEntity() { void TRestGDMLParser::ReplaceAttributeWithKeyWord(string keyword) { int n; - while ((n = filestr.find(keyword, 0)) != -1) { + while ((n = fFileString.find(keyword, 0)) != -1) { int pos1 = 0, pos2 = 0; for (int i = n; i >= 0; i--) { - if (filestr[i] == '"') { + if (fFileString[i] == '"') { pos1 = i + 1; break; } } - for (unsigned int i = n; i < filestr.size(); i++) { - if (filestr[i] == '"') { + for (unsigned int i = n; i < fFileString.size(); i++) { + if (fFileString[i] == '"') { pos2 = i; break; } } - string target = filestr.substr(pos1, pos2 - pos1); + string target = fFileString.substr(pos1, pos2 - pos1); // cout << target << endl; string replace = ReplaceMathematicalExpressions(ReplaceConstants(ReplaceVariables(target))); // cout << replace << endl; @@ -182,6 +178,6 @@ void TRestGDMLParser::ReplaceAttributeWithKeyWord(string keyword) { cout << replace << endl; exit(0); } - filestr.replace(pos1, pos2 - pos1, replace); + fFileString.replace(pos1, pos2 - pos1, replace); } } From 1c6fce370884bba53730737843b5b82477b13410 Mon Sep 17 00:00:00 2001 From: lobis Date: Mon, 9 May 2022 19:00:36 +0200 Subject: [PATCH 03/16] TRestGDMLParser - `fOutputGdmlDirectory` now uses "/tmp/gdml" if REST user is not defined, to prevent error --- source/framework/core/inc/TRestGDMLParser.h | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/source/framework/core/inc/TRestGDMLParser.h b/source/framework/core/inc/TRestGDMLParser.h index 52fc92b07..44accacc7 100644 --- a/source/framework/core/inc/TRestGDMLParser.h +++ b/source/framework/core/inc/TRestGDMLParser.h @@ -7,23 +7,22 @@ #include "TRestMetadata.h" -/////////////////////////////////////////// // we must preprocess gdml file because of a bug in TGDMLParse::Value() in ROOT6 -// class TRestGDMLParser : public TRestMetadata { private: - TGeoManager* fGeoManager; + TGeoManager* fGeoManager{}; public: - TRestGDMLParser() {} - ~TRestGDMLParser() {} - std::string fFileString; - std::string fPath; - std::string fOutputGdmlDirectory = REST_USER_PATH + "/gdml/"; - std::string fOutputGdmlFilename; + TRestGDMLParser() = default; + ~TRestGDMLParser() = default; + + std::string fFileString = ""; + std::string fPath = ""; + std::string fOutputGdmlDirectory = REST_USER_PATH.empty() ? "/tmp/gdml/" : REST_USER_PATH + "/gdml/"; + std::string fOutputGdmlFilename = ""; std::string fGdmlVersion = "0.0"; - std::map fEntityVersionMap; + std::map fEntityVersionMap{}; std::string GetEntityVersion(const std::string& name) const; @@ -41,7 +40,7 @@ class TRestGDMLParser : public TRestMetadata { TGeoManager* CreateGeoM(); - void InitFromConfigFile() {} + void InitFromConfigFile() override {} void PrintContent(); From d23d41f68126195915f388b41a8920ff804094d2 Mon Sep 17 00:00:00 2001 From: lobis Date: Mon, 9 May 2022 19:01:33 +0200 Subject: [PATCH 04/16] TRestGDMLParser - using class name in prints --- source/framework/core/src/TRestGDMLParser.cxx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/source/framework/core/src/TRestGDMLParser.cxx b/source/framework/core/src/TRestGDMLParser.cxx index 9430b6b74..7a6a15fdc 100644 --- a/source/framework/core/src/TRestGDMLParser.cxx +++ b/source/framework/core/src/TRestGDMLParser.cxx @@ -31,7 +31,7 @@ void TRestGDMLParser::Load(const string& filename) { fFileString = ReplaceConstants(ReplaceVariables(fFileString)); - cout << "GDML: initializating variables" << endl; + cout << "TRestGDMLParser: initializing variables" << endl; int pos = fFileString.find(" Date: Mon, 9 May 2022 19:11:08 +0200 Subject: [PATCH 05/16] TRestGDMLParser - renamed `CreateGeoM` method to `CreateGeoManager`, made error exists return error code --- source/framework/core/inc/TRestGDMLParser.h | 6 +-- source/framework/core/src/TRestGDMLParser.cxx | 49 ++++++++----------- 2 files changed, 24 insertions(+), 31 deletions(-) diff --git a/source/framework/core/inc/TRestGDMLParser.h b/source/framework/core/inc/TRestGDMLParser.h index 44accacc7..b0ef4b062 100644 --- a/source/framework/core/inc/TRestGDMLParser.h +++ b/source/framework/core/inc/TRestGDMLParser.h @@ -38,7 +38,7 @@ class TRestGDMLParser : public TRestMetadata { return fGeoManager; } - TGeoManager* CreateGeoM(); + TGeoManager* CreateGeoManager(); void InitFromConfigFile() override {} @@ -46,9 +46,9 @@ class TRestGDMLParser : public TRestMetadata { void ReplaceEntity(); - void ReplaceAttributeWithKeyWord(std::string keyword); + void ReplaceAttributeWithKeyWord(const std::string& keyword); - ClassDef(TRestGDMLParser, 2); + ClassDefOverride(TRestGDMLParser, 2); }; #endif diff --git a/source/framework/core/src/TRestGDMLParser.cxx b/source/framework/core/src/TRestGDMLParser.cxx index 7a6a15fdc..be2c3fd76 100644 --- a/source/framework/core/src/TRestGDMLParser.cxx +++ b/source/framework/core/src/TRestGDMLParser.cxx @@ -34,9 +34,9 @@ void TRestGDMLParser::Load(const string& filename) { cout << "TRestGDMLParser: initializing variables" << endl; int pos = fFileString.find("(t)), std::istreambuf_iterator()); t.close(); @@ -133,12 +133,11 @@ void TRestGDMLParser::ReplaceEntity() { } } - // cout << "replacing entity..." << endl; fFileString.replace(pos5, entityName.length() + 2, str); } else { cout << "GDML ERROR! No matching reference file for entity: \"" << entityName << "\"" << endl; - cout << "file name: \"" << entityfile << "\"" << endl; - exit(0); + cout << "file name: \"" << entityFile << "\"" << endl; + exit(1); } } else { cout << "TRestGDMLParser: Warning! redundant entity: \"" << entityName << "\"" << endl; @@ -147,7 +146,7 @@ void TRestGDMLParser::ReplaceEntity() { } } -void TRestGDMLParser::ReplaceAttributeWithKeyWord(string keyword) { +void TRestGDMLParser::ReplaceAttributeWithKeyWord(const string& keyword) { int n; while ((n = fFileString.find(keyword, 0)) != -1) { int pos1 = 0, pos2 = 0; @@ -165,18 +164,12 @@ void TRestGDMLParser::ReplaceAttributeWithKeyWord(string keyword) { } } string target = fFileString.substr(pos1, pos2 - pos1); - // cout << target << endl; string replace = ReplaceMathematicalExpressions(ReplaceConstants(ReplaceVariables(target))); - // cout << replace << endl; - // cout << target << " " << ReplaceConstants(ReplaceVariables(target) << " " - // << replace << endl; if (replace == target) { - cout << "Error! failed to replace mathematical expressions! check the " - "file!" - << endl; + cout << "Error! failed to replace mathematical expressions! check the file!" << endl; cout << replace << endl; - exit(0); + exit(1); } fFileString.replace(pos1, pos2 - pos1, replace); } From bc07e790d2500f9b3ea9986a345f3cf7688c661a Mon Sep 17 00:00:00 2001 From: lobis Date: Mon, 9 May 2022 19:33:52 +0200 Subject: [PATCH 06/16] TRestGDMLParser - added header to source file. Check GDML file is created, create necessary directories if they do not exist --- source/framework/core/src/TRestGDMLParser.cxx | 44 ++++++++++++------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/source/framework/core/src/TRestGDMLParser.cxx b/source/framework/core/src/TRestGDMLParser.cxx index be2c3fd76..2aab431df 100644 --- a/source/framework/core/src/TRestGDMLParser.cxx +++ b/source/framework/core/src/TRestGDMLParser.cxx @@ -1,5 +1,7 @@ #include "TRestGDMLParser.h" +#include + using namespace std; string TRestGDMLParser::GetEntityVersion(const string& name) const { @@ -31,7 +33,7 @@ void TRestGDMLParser::Load(const string& filename) { fFileString = ReplaceConstants(ReplaceVariables(fFileString)); - cout << "TRestGDMLParser: initializing variables" << endl; + cout << "TRestGDMLParser: Initializing variables" << endl; int pos = fFileString.find("Import(fOutputGdmlFilename.c_str()); + auto geoManager = new TGeoManager(); + geoManager->Import(fOutputGdmlFilename.c_str()); chdir(originDirectory); - return geo2; + return geoManager; } return nullptr; } @@ -94,13 +104,12 @@ void TRestGDMLParser::ReplaceEntity() { int pos1 = fFileString.find_first_not_of(" ", pos + 8); int pos2 = fFileString.find("SYSTEM", pos1); string entityName = RemoveWhiteSpaces(fFileString.substr(pos1, pos2 - pos1)); - // cout << entityName << endl; int pos3 = fFileString.find("\"", pos2) + 1; int pos4 = fFileString.find("\"", pos3); string entityFile = RemoveWhiteSpaces(fFileString.substr(pos3, pos4 - pos3)); - cout << "TRestGDMLParser: replacing entity: " << entityName << ", file: " << entityFile << endl; + cout << "TRestGDMLParser: Replacing entity: " << entityName << ", file: " << entityFile << endl; if ((int)entityFile.find("http") != -1) { string entityField = @@ -118,9 +127,10 @@ void TRestGDMLParser::ReplaceEntity() { int pos5 = 0; if ((pos5 = fFileString.find("&" + entityName + ";")) != -1) { if (TRestTools::fileExists(entityFile)) { - std::ifstream t(entityFile); - std::string str((std::istreambuf_iterator(t)), std::istreambuf_iterator()); - t.close(); + std::ifstream entityFileRead(entityFile); + std::string str((std::istreambuf_iterator(entityFileRead)), + std::istreambuf_iterator()); + entityFileRead.close(); str = ReplaceConstants(ReplaceVariables(str)); From 90eaa64695e494fd6e96a116ee8de03d96f27e30 Mon Sep 17 00:00:00 2001 From: lobis Date: Mon, 9 May 2022 21:05:12 +0200 Subject: [PATCH 07/16] TRestGDMLParser - restored removed `CreateGeoM` method and added deprecation warning --- source/framework/core/inc/TRestGDMLParser.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/source/framework/core/inc/TRestGDMLParser.h b/source/framework/core/inc/TRestGDMLParser.h index b0ef4b062..ae7736ab0 100644 --- a/source/framework/core/inc/TRestGDMLParser.h +++ b/source/framework/core/inc/TRestGDMLParser.h @@ -39,6 +39,9 @@ class TRestGDMLParser : public TRestMetadata { } TGeoManager* CreateGeoManager(); + [[deprecated("Use CreateGeoManager() instead.")]] inline TGeoManager* CreateGeoM() { + return CreateGeoManager(); + } void InitFromConfigFile() override {} From d58379bd5a2f7cff6e46e307f57c2b167aaf7f79 Mon Sep 17 00:00:00 2001 From: lobis Date: Mon, 9 May 2022 21:20:45 +0200 Subject: [PATCH 08/16] Replaced `Nevents` by `nEvents` globally (old way still works) --- pipeline/pandaxiii_MC/Xe136bb0n.rml | 4 ++-- pipeline/pandaxiii_MC/geometry/Th232_SS_01.rml | 2 +- source/framework/tools/src/TRestStringHelper.cxx | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pipeline/pandaxiii_MC/Xe136bb0n.rml b/pipeline/pandaxiii_MC/Xe136bb0n.rml index e7ec239a9..60a007bea 100644 --- a/pipeline/pandaxiii_MC/Xe136bb0n.rml +++ b/pipeline/pandaxiii_MC/Xe136bb0n.rml @@ -22,8 +22,8 @@ - - + + diff --git a/pipeline/pandaxiii_MC/geometry/Th232_SS_01.rml b/pipeline/pandaxiii_MC/geometry/Th232_SS_01.rml index c1deb8f5d..531f98fdc 100644 --- a/pipeline/pandaxiii_MC/geometry/Th232_SS_01.rml +++ b/pipeline/pandaxiii_MC/geometry/Th232_SS_01.rml @@ -26,7 +26,7 @@ - + diff --git a/source/framework/tools/src/TRestStringHelper.cxx b/source/framework/tools/src/TRestStringHelper.cxx index b8ea5b19a..cc973e90a 100644 --- a/source/framework/tools/src/TRestStringHelper.cxx +++ b/source/framework/tools/src/TRestStringHelper.cxx @@ -486,7 +486,7 @@ Float_t REST_StringHelper::StringToFloat(string in) { /// \brief Gets an integer from a string. /// Int_t REST_StringHelper::StringToInteger(string in) { - // If we find an hexadecimal number + // If we find a hexadecimal number if (in.find("0x") != std::string::npos) return (Int_t)std::stoul(in, nullptr, 16); return (Int_t)StringToDouble(in); From 1ffc4f5996a910985bf1a4c474d793a41593aae1 Mon Sep 17 00:00:00 2001 From: lobis Date: Mon, 9 May 2022 21:27:27 +0200 Subject: [PATCH 09/16] Globally replaced `gdml_file` by `gdmlFile` (old way still works) --- pipeline/pandaxiii_MC/Xe136bb0n.rml | 2 +- pipeline/pandaxiii_MC/geometry/Th232_SS_01.rml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pipeline/pandaxiii_MC/Xe136bb0n.rml b/pipeline/pandaxiii_MC/Xe136bb0n.rml index 60a007bea..01abdab55 100644 --- a/pipeline/pandaxiii_MC/Xe136bb0n.rml +++ b/pipeline/pandaxiii_MC/Xe136bb0n.rml @@ -24,7 +24,7 @@ - + diff --git a/pipeline/pandaxiii_MC/geometry/Th232_SS_01.rml b/pipeline/pandaxiii_MC/geometry/Th232_SS_01.rml index 531f98fdc..280c38e7f 100644 --- a/pipeline/pandaxiii_MC/geometry/Th232_SS_01.rml +++ b/pipeline/pandaxiii_MC/geometry/Th232_SS_01.rml @@ -27,7 +27,7 @@ - + From 3fa057388127053b1fe13f317e6605c35b6c8421 Mon Sep 17 00:00:00 2001 From: lobis Date: Mon, 9 May 2022 22:38:22 +0200 Subject: [PATCH 10/16] TRestMetadata - fix some typos --- source/framework/core/src/TRestMetadata.cxx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/source/framework/core/src/TRestMetadata.cxx b/source/framework/core/src/TRestMetadata.cxx index 1f4253806..4a3d90574 100644 --- a/source/framework/core/src/TRestMetadata.cxx +++ b/source/framework/core/src/TRestMetadata.cxx @@ -113,18 +113,18 @@ /// If no arguments are provided, LoadConfigFromFile() will only call the Initialize() /// method. If given the rml file name, it will find out the needed rml sections. We /// can also directly give the xml sections to the method. Two xml sections are used -/// to startup the class: the section for the class and a global section. Additionaly +/// to startup the class: the section for the class and a global section. Additionally /// we can give a map object to the method to import additional variables. /// /// The "section for the class" is an xml section with the value of class name. -/// It is the main information souce for the class's startup. The "global" +/// It is the main information source for the class's startup. The "global" /// section is a special xml section in the rml file, containing global information /// which could be seen by all the class sections. /// /// With the xml sections given, LoadConfigFromFile() first merge them together. Then it /// calls LoadSectionMetadata(), which loads some universal parameters like /// name, title and verbose level. This method also preprocesses the config -/// sections, expanding the include/for decinition and replacing the variables. +/// sections, expanding the include/for definition and replacing the variables. /// After this, LoadConfigFromFile() calls the method InitFromConfigFile(). /// /// **InitFromConfigFile()** is a pure virtual method and every child classes have @@ -1269,7 +1269,7 @@ void TRestMetadata::ExpandIncludeFile(TiXmlElement* e) { /// If still not found, it returns the default value. /// /// \param parName The name of the parameter from which we want to obtain the -/// value. \param defaultValue The default value if the paremeter is not found +/// value. \param defaultValue The default value if the parameter is not found /// /// \return A string of result string TRestMetadata::GetParameter(std::string parName, TString defaultValue) { @@ -1307,7 +1307,7 @@ string TRestMetadata::GetParameter(std::string parName, TString defaultValue) { /// /// \param parName The name of the parameter from which we want to obtain the /// value. \param e The target eml element where the program is to search the -/// parameter \param defaultValue The default value if the paremeter is not +/// parameter \param defaultValue The default value if the parameter is not /// found /// /// \return A string of result, with env and expressions replaced From 0d73a2b3719bda996520d6a2ac69b1236302f6b4 Mon Sep 17 00:00:00 2001 From: juanan Date: Fri, 6 May 2022 12:48:03 +0200 Subject: [PATCH 11/16] Removing unnecesary stuff in Testing.cmake --- cmake/Testing.cmake | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/cmake/Testing.cmake b/cmake/Testing.cmake index 12a40b7fa..e021c12c4 100644 --- a/cmake/Testing.cmake +++ b/cmake/Testing.cmake @@ -9,13 +9,6 @@ if (TEST) add_compile_definitions(REST_TESTING_ENABLED) endif () -macro(ADD_TEST) - if (TEST) - message(STATUS "Adding tests at ${CMAKE_CURRENT_SOURCE_DIR}") - add_subdirectory(test) - endif () -endmacro() - macro(ADD_LIBRARY_TEST) if (TEST) message(STATUS "Adding tests at ${CMAKE_CURRENT_SOURCE_DIR}") @@ -31,13 +24,9 @@ macro(ADD_LIBRARY_TEST) enable_testing() - add_executable(${TESTING_EXECUTABLE}) - FILE(GLOB SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/test/src/*.cxx) - target_sources( - ${TESTING_EXECUTABLE} PUBLIC - ${SOURCES} - ) + + add_executable(${TESTING_EXECUTABLE} ${SOURCES}) target_link_libraries( ${TESTING_EXECUTABLE} PUBLIC From 172ba772c9d444c913ff66af725cb788b47598ad Mon Sep 17 00:00:00 2001 From: juanan Date: Fri, 6 May 2022 12:50:08 +0200 Subject: [PATCH 12/16] Replacing gtest_discover_tests by gtest_add_tests to avoid errors during compilation time --- cmake/Testing.cmake | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cmake/Testing.cmake b/cmake/Testing.cmake index e021c12c4..32c756b5d 100644 --- a/cmake/Testing.cmake +++ b/cmake/Testing.cmake @@ -38,7 +38,9 @@ macro(ADD_LIBRARY_TEST) include(GoogleTest) - gtest_discover_tests(${TESTING_EXECUTABLE}) - + gtest_add_tests( + TARGET ${TESTING_EXECUTABLE} + SOURCES ${SOURCES} + ) endif () endmacro() From 887ef172c03815acebbd6ece9f32af8dabcb58ef Mon Sep 17 00:00:00 2001 From: juanan Date: Fri, 6 May 2022 12:51:56 +0200 Subject: [PATCH 13/16] Bug fix loadG4 macro while generating thisREST.sh --- cmake/thisREST.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmake/thisREST.cmake b/cmake/thisREST.cmake index 83f60c3dc..d96f0539e 100644 --- a/cmake/thisREST.cmake +++ b/cmake/thisREST.cmake @@ -15,7 +15,7 @@ string(REGEX REPLACE "\n$" "" GEANT4_PATH "${GEANT4_PATH}") set(thisGeant4 "${GEANT4_PATH}/bin/geant4.sh") if (${REST_G4} MATCHES "ON") - set(loadG4 "\# if geant4.sh script is found we load the same Geant4 version as used in compilation\n if [[ -f \"${thisGeant4}\" && ${thisGeant4} != /usr/* ]]; then + set(loadG4 "\# if geant4.sh script is found we load the same Geant4 version as used in compilation\n if [[ -f \\\"${thisGeant4}\\\" && ${thisGeant4} != /usr/* ]]; then source ${thisGeant4}\n fi\n") else () set(loadG4 "") From c497cb5047f7abcbac117337f4fdc95f74988136 Mon Sep 17 00:00:00 2001 From: lobis Date: Tue, 10 May 2022 11:20:15 +0200 Subject: [PATCH 14/16] .gitlab-ci.yml - added test output on failure only --- .gitlab-ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index c995d191e..f1edaace7 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -87,7 +87,7 @@ Build and Test: - cmake ${CI_PROJECT_DIR} -DTEST=ON -DREST_GARFIELD=OFF -DREST_G4=ON -DRESTLIB_DETECTOR=ON -DRESTLIB_RAW=ON -DRESTLIB_TRACK=ON - make -j2 - - ctest --verbose -O ${CI_PROJECT_DIR}/build/Testing/summary.txt + - ctest --output-on-failure -O ${CI_PROJECT_DIR}/build/Testing/summary.txt artifacts: name: "Testing" From 26647a2e339a480acc8cca89774f3a3c50ab5284 Mon Sep 17 00:00:00 2001 From: lobis Date: Tue, 10 May 2022 12:37:15 +0200 Subject: [PATCH 15/16] Not using `#define` in tests when possible --- source/framework/test/src/FrameworkCore.cxx | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/source/framework/test/src/FrameworkCore.cxx b/source/framework/test/src/FrameworkCore.cxx index 960663bba..dbce168e3 100644 --- a/source/framework/test/src/FrameworkCore.cxx +++ b/source/framework/test/src/FrameworkCore.cxx @@ -9,26 +9,26 @@ namespace fs = std::filesystem; using namespace std; -#define FILES_PATH fs::path(__FILE__).parent_path().parent_path() / "files" -#define BASIC_TRESTRUN_RML FILES_PATH / "TRestRunBasic.rml" -#define BASIC_TRESTMETADATA_RML FILES_PATH / "TRestMetadataTest.rml" +const auto filesPath = fs::path(__FILE__).parent_path().parent_path() / "files"; +const auto basicRunRml = filesPath / "TRestRunBasic.rml"; +const auto basicMetadataRml = filesPath / "TRestMetadataTest.rml"; TEST(FrameworkCore, TestFiles) { - cout << "FrameworkCore test files path: " << FILES_PATH << endl; + cout << "FrameworkCore test files path: " << filesPath << endl; // Check dir exists and is a directory - EXPECT_TRUE(fs::is_directory(FILES_PATH)); + EXPECT_TRUE(fs::is_directory(filesPath)); // Check it's not empty - EXPECT_TRUE(!fs::is_empty(FILES_PATH)); + EXPECT_TRUE(!fs::is_empty(filesPath)); // Check required files exist - EXPECT_TRUE(fs::exists(BASIC_TRESTRUN_RML)); - EXPECT_TRUE(fs::exists(BASIC_TRESTMETADATA_RML)); + EXPECT_TRUE(fs::exists(basicRunRml)); + EXPECT_TRUE(fs::exists(basicMetadataRml)); } TEST(FrameworkCore, TRestRun) { TRestRun run; - run.LoadConfigFromFile(BASIC_TRESTRUN_RML); + run.LoadConfigFromFile(basicRunRml); run.PrintAllMetadata(); @@ -50,7 +50,7 @@ TEST(FrameworkCore, TRestMetadata) { }; TRestMetadataTest restMetadataTest; - restMetadataTest.LoadConfigFromFile(BASIC_TRESTMETADATA_RML, "TRestMetadataTest"); + restMetadataTest.LoadConfigFromFile(basicMetadataRml, "TRestMetadataTest"); restMetadataTest.PrintMetadata(); From ffb4790beeb30cc8be0e30e17c05e10c9a879e25 Mon Sep 17 00:00:00 2001 From: lobis Date: Tue, 10 May 2022 12:38:21 +0200 Subject: [PATCH 16/16] Fix some typos, inline methods --- source/framework/core/src/TRestMetadata.cxx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/source/framework/core/src/TRestMetadata.cxx b/source/framework/core/src/TRestMetadata.cxx index 75833c35c..9a82722c9 100644 --- a/source/framework/core/src/TRestMetadata.cxx +++ b/source/framework/core/src/TRestMetadata.cxx @@ -515,8 +515,8 @@ TRestMetadata::TRestMetadata(const char* cfgFileName) : endl(this) { /// \brief TRestMetadata default destructor /// TRestMetadata::~TRestMetadata() { - if (fElementGlobal) delete fElementGlobal; - if (fElement) delete fElement; + delete fElementGlobal; + delete fElement; } /////////////////////////////////////////////// @@ -564,7 +564,7 @@ Int_t TRestMetadata::LoadConfigFromFile(string cfgFileName, string sectionName) delete rootEle; return result; } else { - ferr << "Filename : " << fConfigFileName << endl; + ferr << "Filename: " << fConfigFileName << endl; ferr << "Config File does not exist. Right path/filename?" << endl; GetChar(); return -1; @@ -628,7 +628,7 @@ Int_t TRestMetadata::LoadConfigFromBuffer() { /////////////////////////////////////////////// /// \brief This method does some preparation of xml section. /// -/// Preparation includes: seting the name, title and verbose level of the +/// Preparation includes: setting the name, title and verbose level of the /// current class. Finding out and saving the env sections. /// /// By calling TRestMetadata::ReadElement(), is also expands for loops and