[packchk] reviewed error messages (#590)

* reviewed error messages * fixed test cases
Open-CMSIS-Pack · Jun 6, 2023 · b2d0d7b · b2d0d7b
1 parent 628c8c3
commit b2d0d7b
Show file tree

Hide file tree

Showing 5 changed files with 63 additions and 78 deletions.
diff --git a/tools/packchk/src/CreateModel.cpp b/tools/packchk/src/CreateModel.cpp
@@ -157,7 +157,7 @@ bool CreateModel::SetPackXsd(const std::string& packXsdFile)
   }
 
   if(!RteFsUtils::Exists(packXsdFile)) {
-    LogMsg("M218", PATH(packXsdFile));
+    LogMsg("M219", PATH(packXsdFile));
     return false;
   }
 

diff --git a/tools/packchk/src/PackChk.cpp b/tools/packchk/src/PackChk.cpp
@@ -174,27 +174,30 @@ bool PackChk::CheckPackage()
 */
 int PackChk::Check(int argc, const char* argv[], const char* envp[])
 {
+  const string header = m_packOptions.GetHeader();
+  LogMsg("M001", TXT(header));
+
   ParseOptions parseOptions(m_packOptions);
   ParseOptions::Result result = parseOptions.Parse(argc, argv);
+
+  // Add date and time to log file
+  if(!m_packOptions.GetLogPath().empty()) {
+    string dateTime = m_packOptions.GetCurrentDateTime();
+    LogMsg("M002", TXT("Log created on "), TXT2(dateTime));
+  }
+
   switch(result) {
     case ParseOptions::Result::Ok:
       break;
     case ParseOptions::Result::ExitNoError:
       return 0;
     case ParseOptions::Result::Error:
-      LogMsg("M105");
+      if(!ErrLog::Get()->GetErrCnt()) {
+        LogMsg("M105");
+      }
       return 1;
   }
 
-  const string header = m_packOptions.GetHeader();
-  LogMsg("M001", TXT(header));
-
-  // Add date and time to log file
-  if(!m_packOptions.GetLogPath().empty()) {
-    string dateTime = m_packOptions.GetCurrentDateTime();
-    LogMsg("M002", TXT("Log created on "), TXT2(dateTime));
-  }
-
   bool bOk = CheckPackage();
 
   if(ErrLog::Get()->GetErrCnt() || !bOk) {

diff --git a/tools/packchk/src/PackChk_Msgs.cpp b/tools/packchk/src/PackChk_Msgs.cpp
@@ -101,7 +101,8 @@ const MsgTable PackChk::msgTable = {
   { "M215", { MsgLevel::LEVEL_ERROR,    CRLF_BE, "" } },
   { "M216", { MsgLevel::LEVEL_ERROR,    CRLF_B, "Unable to get executable path %MSG%!"} },
   { "M217", { MsgLevel::LEVEL_ERROR,    CRLF_B, ""} },
-  { "M218", { MsgLevel::LEVEL_ERROR,    CRLF_B, "Unable to find schema file for validation: '%PATH%'"} },
+  { "M218", { MsgLevel::LEVEL_ERROR,    CRLF_B, "Unable to find PACK.xsd schema file. Searched in '%MSG%' relative to '%PATH%'.\n  Alternatively use --xsd to specify the location of the schema file. "} },
+  { "M219", { MsgLevel::LEVEL_ERROR,    CRLF_B, "Unable to find specified schema file '%PATH%'"} },
 
 // 300... Validation Errors
   { "M300", { MsgLevel::LEVEL_ERROR,    CRLF_B, "" } },

diff --git a/tools/packchk/src/PackOptions.cpp b/tools/packchk/src/PackOptions.cpp
@@ -235,11 +235,20 @@ bool CPackOptions::SetXsdFile()
     LogMsg("M216", MSG(ec.message()));
     return false;
   }
+
+  string msgPath;
+
   // Search schema in priority order
   vector<string> relSearchOrder = { "./", "../etc/", "../../etc/" };
   string schemaFilePath;
   for (auto& relPath : relSearchOrder) {
     schemaFilePath = exePath + relPath + "PACK.xsd";
+
+    if(!msgPath.empty()) {
+      msgPath += ", ";
+    }
+    msgPath += relPath;
+
     if (RteFsUtils::Exists(schemaFilePath)) {
       m_xsdPath = fs::canonical(schemaFilePath, ec).generic_string();
       if (m_xsdPath.empty()) {
@@ -250,7 +259,7 @@ bool CPackOptions::SetXsdFile()
     }
   }
 
-  LogMsg("M218", PATH("./, ../etc/, ../../etc/"));
+  LogMsg("M218", PATH(exePath), MSG(msgPath));
 
   return false;
 }
@@ -268,7 +277,7 @@ bool CPackOptions::SetXsdFile(const string& xsdFile)
 
   m_xsdPath = RteFsUtils::AbsolutePath(xsdFile).generic_string();
   if(!RteFsUtils::Exists(m_xsdPath)) {
-    LogMsg("M218", PATH(m_xsdPath));
+    LogMsg("M219", PATH(m_xsdPath));
     m_xsdPath.clear();
 
     return false;

diff --git a/tools/packchk/test/integtests/src/PackChkIntegTests.cpp b/tools/packchk/test/integtests/src/PackChkIntegTests.cpp
@@ -647,14 +647,48 @@ TEST_F(PackChkIntegTests, CheckNotExistXsd) {
   bool bFound = false;
   for (const string& msg : errMsgs) {
     size_t s;
-    if ((s = msg.find("M218")) != string::npos) {
+    if ((s = msg.find("M219")) != string::npos) {
       bFound = true;
       break;
     }
   }
 
   if (!bFound) {
-    FAIL() << "error: missing error M218";
+    FAIL() << "error: missing error M219";
+  }
+}
+
+TEST_F(PackChkIntegTests, CheckPackNamedXsdNotFound) {
+  const char* argv[4];
+
+  const string& pdscFile = PackChkIntegTestEnv::localtestdata_dir +
+    "/SchemaValidation/TestVendor.SchemaValidation.pdsc";
+  ASSERT_TRUE(RteFsUtils::Exists(pdscFile));
+
+  DeletePackXsd();
+
+  const string schemaFileName = GetPackXsd();
+
+  argv[0] = (char*)"";
+  argv[1] = (char*)pdscFile.c_str();
+  argv[2] = (char*)"--xsd";
+  argv[3] = (char*)schemaFileName.c_str();
+
+  PackChk packChk;
+  EXPECT_EQ(1, packChk.Check(4, argv, nullptr));
+
+  auto errMsgs = ErrLog::Get()->GetLogMessages();
+  int M219_foundCnt = 0;
+
+  for (const string& msg : errMsgs) {
+    size_t s;
+    if ((s = msg.find("M219")) != string::npos) {
+      M219_foundCnt++;    // follows one M511: <descripton>
+    }
+  }
+
+  if (M219_foundCnt != 1) {
+    FAIL() << "error: missing message M219";
   }
 }
 
@@ -756,68 +790,6 @@ TEST_F(PackChkIntegTests, CheckSchemaValidation) {
   }
 }
 
-TEST_F(PackChkIntegTests, CheckPackXsdNotFound) {
-  const char* argv[2];
-
-  const string& pdscFile = PackChkIntegTestEnv::localtestdata_dir +
-    "/SchemaValidation/TestVendor.SchemaValidation.pdsc";
-  ASSERT_TRUE(RteFsUtils::Exists(pdscFile));
-
-  DeletePackXsd();
-
-  argv[0] = (char*)"";
-  argv[1] = (char*)pdscFile.c_str();
-
-  PackChk packChk;
-  EXPECT_EQ(1, packChk.Check(2, argv, nullptr));
-
-  auto errMsgs = ErrLog::Get()->GetLogMessages();
-  int M218_foundCnt = 0;
-
-  for (const string& msg : errMsgs) {
-    size_t s;
-    if ((s = msg.find("M218")) != string::npos) {
-      M218_foundCnt++;    // follows one M511: <descripton>
-    }
-  }
-
-  if (M218_foundCnt != 1) {
-    FAIL() << "error: missing message M218";
-  }
-}
-
-TEST_F(PackChkIntegTests, CheckPackNamedXsdNotFound) {
-  const char* argv[4];
-
-  const string& pdscFile = PackChkIntegTestEnv::localtestdata_dir +
-    "/SchemaValidation/TestVendor.SchemaValidation.pdsc";
-  ASSERT_TRUE(RteFsUtils::Exists(pdscFile));
-
-  DeletePackXsd();
-
-  const string schemaFileName = GetPackXsd();
-
-  argv[0] = (char*)"";
-  argv[1] = (char*)pdscFile.c_str();
-  argv[2] = (char*)"--xsd";
-  argv[3] = (char*)schemaFileName.c_str();
-
-  PackChk packChk;
-  EXPECT_EQ(1, packChk.Check(4, argv, nullptr));
-
-  auto errMsgs = ErrLog::Get()->GetLogMessages();
-  int M218_foundCnt = 0;
-
-  for (const string& msg : errMsgs) {
-    size_t s;
-    if ((s = msg.find("M218")) != string::npos) {
-      M218_foundCnt++;    // follows one M511: <descripton>
-    }
-  }
 
-  if (M218_foundCnt != 1) {
-    FAIL() << "error: missing message M218";
-  }
-}