Sanity check the test name and metrics
Some dashboards, and libraries don't support metrics name
other than word character. Add a check to make sure any change went
through would caught by CQ.
BUG=b:172227944
TEST=./vkbench
Change-Id: If9f5eb26f089a3b454d2cbca9c1d761557a44c62
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vkbench/+/2552100
Reviewed-by: Ilja H. Friedel <ihf@chromium.org>
Commit-Queue: Po-Hsien Wang <pwang@chromium.org>
Tested-by: Po-Hsien Wang <pwang@chromium.org>
Auto-Submit: Po-Hsien Wang <pwang@chromium.org>
diff --git a/src/main.cc b/src/main.cc
index 7002d6b..b7899c4 100644
--- a/src/main.cc
+++ b/src/main.cc
@@ -7,6 +7,7 @@
#include <algorithm>
#include <iterator>
#include <map>
+#include <regex>
#include <vector>
#include "filepath.h"
@@ -120,6 +121,9 @@
--verbose Show verbose messages.
--vlayer Enable vulkan verification layer.
--hasty Enable hasty mode.
+ In this mode, tests will try to reuse existing vulkan
+ instance if possible. It also runs a reduced number
+ of tests and checks the validity of each tests.
--spirv_dir Path to SPIRV code for test.(default: shaders/)
--out_dir Path to the output directory.),")
}
@@ -165,18 +169,17 @@
return false;
}
- all_tests.erase(
- remove_if(all_tests.begin(), all_tests.end(),
- [enabled_tests, disabled_tests](const vkbench::testBase* test) {
- bool should_run = enabled_tests.empty() ||
- prefixFind(enabled_tests, test->Name());
- should_run &= !prefixFind(disabled_tests, test->Name());
- if (!should_run)
- delete test;
- return !should_run;
- }),
- all_tests.end());
-
+ auto filterTests = [enabled_tests,
+ disabled_tests](const vkbench::testBase* test) {
+ bool should_run =
+ enabled_tests.empty() || prefixFind(enabled_tests, test->Name());
+ should_run &= !prefixFind(disabled_tests, test->Name());
+ if (!should_run)
+ delete test;
+ return !should_run;
+ };
+ all_tests.erase(remove_if(all_tests.begin(), all_tests.end(), filterTests),
+ all_tests.end());
if (g_list) {
for (const auto& test : all_tests) {
LOG("%s: %s", test->Name(), test->Desp())
@@ -201,6 +204,16 @@
return 0;
}
+ // Sanity check as certain characters is not allowed by dashboard.
+ auto assertValidCharater = [](const char* str) {
+ if (!regex_match(std::string(str), std::regex(R"(\w+)")))
+ RUNTIME_ERROR("%s contains invalid character", str);
+ };
+ for (auto test : all_tests) {
+ assertValidCharater(test->Name());
+ assertValidCharater(test->Unit());
+ }
+
// Sort to bundle tests using same vulkan instance together.
std::stable_sort(all_tests.begin(), all_tests.end(),
[](vkbench::testBase* a, vkbench::testBase* b) -> bool {
diff --git a/src/tests/drawSizeTest.h b/src/tests/drawSizeTest.h
index f69e331..d5f3646 100644
--- a/src/tests/drawSizeTest.h
+++ b/src/tests/drawSizeTest.h
@@ -13,7 +13,7 @@
DrawSizeTest(uint64_t size, vkBase* base) {
vk = base;
img_size_.setHeight(size).setWidth(size);
- snprintf(name_, 100, "DrawSizeTest@%u", img_size_.height);
+ snprintf(name_, 100, "DrawSizeTest_%u", img_size_.height);
snprintf(desp_, 1024, "Time to draw %ux%u image.", img_size_.height,
img_size_.width);
}
diff --git a/src/tests/submitTest.h b/src/tests/submitTest.h
index 8e58620..2dd2a81 100644
--- a/src/tests/submitTest.h
+++ b/src/tests/submitTest.h
@@ -12,7 +12,7 @@
public:
SubmitTest(uint64_t submitCnt, vkBase* base) {
vk = base;
- sprintf(name_, "SubmitTest@%lu", submitCnt);
+ sprintf(name_, "SubmitTest_%lu", submitCnt);
sprintf(desp_, "Times the time used when submitting %lu empty calls.",
submitCnt);
smt_infos_.resize(submitCnt);
diff --git a/src/utils.h b/src/utils.h
index e4f2373..5fde169 100644
--- a/src/utils.h
+++ b/src/utils.h
@@ -75,10 +75,12 @@
} else {
sprintf(string_format, "%s\n", format);
}
- vfprintf(fileid, string_format, args);
+ char result[strlen(string_format) + 1024];
+ vsprintf(result, string_format, args);
va_end(args);
+ fprintf(fileid, result);
if (should_abort)
- throw std::runtime_error(string_format);
+ throw std::runtime_error(result);
}
inline void VkCheck(const vk::Result& result, const char* func_str) {