Merge topic 'ctest-resource-allocation-spec-message'

b393b32b4b CTest: Improve error handling when reading resource spec file

Acked-by: Kitware Robot <kwrobot@kitware.com>
Merge-request: !4162
This commit is contained in:
Craig Scott
2019-12-29 00:02:48 +00:00
committed by Kitware Robot
5 changed files with 153 additions and 76 deletions

View File

@@ -16,21 +16,22 @@
static const cmsys::RegularExpression IdentifierRegex{ "^[a-z_][a-z0-9_]*$" };
static const cmsys::RegularExpression IdRegex{ "^[a-z0-9_]+$" };
bool cmCTestResourceSpec::ReadFromJSONFile(const std::string& filename)
cmCTestResourceSpec::ReadFileResult cmCTestResourceSpec::ReadFromJSONFile(
const std::string& filename)
{
cmsys::ifstream fin(filename.c_str());
if (!fin) {
return false;
return ReadFileResult::FILE_NOT_FOUND;
}
Json::Value root;
Json::CharReaderBuilder builder;
if (!Json::parseFromStream(builder, fin, &root, nullptr)) {
return false;
return ReadFileResult::JSON_PARSE_ERROR;
}
if (!root.isObject()) {
return false;
return ReadFileResult::INVALID_ROOT;
}
int majorVersion = 1;
@@ -39,42 +40,42 @@ bool cmCTestResourceSpec::ReadFromJSONFile(const std::string& filename)
auto const& version = root["version"];
if (version.isObject()) {
if (!version.isMember("major") || !version.isMember("minor")) {
return false;
return ReadFileResult::INVALID_VERSION;
}
auto const& major = version["major"];
auto const& minor = version["minor"];
if (!major.isInt() || !minor.isInt()) {
return false;
return ReadFileResult::INVALID_VERSION;
}
majorVersion = major.asInt();
minorVersion = minor.asInt();
} else {
return false;
return ReadFileResult::INVALID_VERSION;
}
} else {
return false;
return ReadFileResult::NO_VERSION;
}
if (majorVersion != 1 || minorVersion != 0) {
return false;
return ReadFileResult::UNSUPPORTED_VERSION;
}
auto const& local = root["local"];
if (!local.isArray()) {
return false;
return ReadFileResult::INVALID_SOCKET_SPEC;
}
if (local.size() > 1) {
return false;
return ReadFileResult::INVALID_SOCKET_SPEC;
}
if (local.empty()) {
this->LocalSocket.Resources.clear();
return true;
return ReadFileResult::READ_OK;
}
auto const& localSocket = local[0];
if (!localSocket.isObject()) {
return false;
return ReadFileResult::INVALID_SOCKET_SPEC;
}
std::map<std::string, std::vector<cmCTestResourceSpec::Resource>> resources;
cmsys::RegularExpressionMatch match;
@@ -88,21 +89,21 @@ bool cmCTestResourceSpec::ReadFromJSONFile(const std::string& filename)
cmCTestResourceSpec::Resource resource;
if (!item.isMember("id")) {
return false;
return ReadFileResult::INVALID_RESOURCE;
}
auto const& id = item["id"];
if (!id.isString()) {
return false;
return ReadFileResult::INVALID_RESOURCE;
}
resource.Id = id.asString();
if (!IdRegex.find(resource.Id.c_str(), match)) {
return false;
return ReadFileResult::INVALID_RESOURCE;
}
if (item.isMember("slots")) {
auto const& capacity = item["slots"];
if (!capacity.isConvertibleTo(Json::uintValue)) {
return false;
return ReadFileResult::INVALID_RESOURCE;
}
resource.Capacity = capacity.asUInt();
} else {
@@ -111,17 +112,55 @@ bool cmCTestResourceSpec::ReadFromJSONFile(const std::string& filename)
r.push_back(resource);
} else {
return false;
return ReadFileResult::INVALID_RESOURCE;
}
}
} else {
return false;
return ReadFileResult::INVALID_RESOURCE_TYPE;
}
}
}
this->LocalSocket.Resources = std::move(resources);
return true;
return ReadFileResult::READ_OK;
}
const char* cmCTestResourceSpec::ResultToString(ReadFileResult result)
{
switch (result) {
case ReadFileResult::READ_OK:
return "OK";
case ReadFileResult::FILE_NOT_FOUND:
return "File not found";
case ReadFileResult::JSON_PARSE_ERROR:
return "JSON parse error";
case ReadFileResult::INVALID_ROOT:
return "Invalid root object";
case ReadFileResult::NO_VERSION:
return "No version specified";
case ReadFileResult::INVALID_VERSION:
return "Invalid version object";
case ReadFileResult::UNSUPPORTED_VERSION:
return "Unsupported version";
case ReadFileResult::INVALID_SOCKET_SPEC:
return "Invalid socket object";
case ReadFileResult::INVALID_RESOURCE_TYPE:
return "Invalid resource type object";
case ReadFileResult::INVALID_RESOURCE:
return "Invalid resource object";
default:
return "Unknown";
}
}
bool cmCTestResourceSpec::operator==(const cmCTestResourceSpec& other) const

View File

@@ -31,7 +31,22 @@ public:
Socket LocalSocket;
bool ReadFromJSONFile(const std::string& filename);
enum class ReadFileResult
{
READ_OK,
FILE_NOT_FOUND,
JSON_PARSE_ERROR,
INVALID_ROOT,
NO_VERSION,
INVALID_VERSION,
UNSUPPORTED_VERSION,
INVALID_SOCKET_SPEC, // Can't be INVALID_SOCKET due to a Windows macro
INVALID_RESOURCE_TYPE,
INVALID_RESOURCE,
};
ReadFileResult ReadFromJSONFile(const std::string& filename);
static const char* ResultToString(ReadFileResult result);
bool operator==(const cmCTestResourceSpec& other) const;
bool operator!=(const cmCTestResourceSpec& other) const;

View File

@@ -537,9 +537,13 @@ bool cmCTestTestHandler::ProcessOptions()
val = this->GetOption("ResourceSpecFile");
if (val) {
this->UseResourceSpec = true;
if (!this->ResourceSpec.ReadFromJSONFile(val)) {
auto result = this->ResourceSpec.ReadFromJSONFile(val);
if (result != cmCTestResourceSpec::ReadFileResult::READ_OK) {
cmCTestLog(this->CTest, ERROR_MESSAGE,
"Could not read resource spec file: " << val << std::endl);
"Could not read/parse resource spec file "
<< val << ": "
<< cmCTestResourceSpec::ResultToString(result)
<< std::endl);
return false;
}
}

View File

@@ -7,71 +7,89 @@
struct ExpectedSpec
{
std::string Path;
bool ParseResult;
cmCTestResourceSpec::ReadFileResult ParseResult;
cmCTestResourceSpec Expected;
};
static const std::vector<ExpectedSpec> expectedResourceSpecs = {
/* clang-format off */
{"spec1.json", true, {{{
{"gpus", {
{"2", 4},
{"e", 1},
}},
{"threads", {
}},
}}}},
{"spec2.json", true, {}},
{"spec3.json", false, {}},
{"spec4.json", false, {}},
{"spec5.json", false, {}},
{"spec6.json", false, {}},
{"spec7.json", false, {}},
{"spec8.json", false, {}},
{"spec9.json", false, {}},
{"spec10.json", false, {}},
{"spec11.json", false, {}},
{"spec12.json", false, {}},
{"spec13.json", false, {}},
{"spec14.json", true, {}},
{"spec15.json", true, {}},
{"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, {}},
{"spec36.json", false, {}},
{"noexist.json", false, {}},
/* clang-format on */
{ "spec1.json",
cmCTestResourceSpec::ReadFileResult::READ_OK,
{ { {
{ "gpus",
{
{ "2", 4 },
{ "e", 1 },
} },
{ "threads", {} },
} } } },
{ "spec2.json", cmCTestResourceSpec::ReadFileResult::READ_OK, {} },
{ "spec3.json",
cmCTestResourceSpec::ReadFileResult::INVALID_SOCKET_SPEC,
{} },
{ "spec4.json",
cmCTestResourceSpec::ReadFileResult::INVALID_SOCKET_SPEC,
{} },
{ "spec5.json",
cmCTestResourceSpec::ReadFileResult::INVALID_SOCKET_SPEC,
{} },
{ "spec6.json",
cmCTestResourceSpec::ReadFileResult::INVALID_SOCKET_SPEC,
{} },
{ "spec7.json",
cmCTestResourceSpec::ReadFileResult::INVALID_RESOURCE_TYPE,
{} },
{ "spec8.json", cmCTestResourceSpec::ReadFileResult::INVALID_RESOURCE, {} },
{ "spec9.json", cmCTestResourceSpec::ReadFileResult::INVALID_RESOURCE, {} },
{ "spec10.json", cmCTestResourceSpec::ReadFileResult::INVALID_RESOURCE, {} },
{ "spec11.json", cmCTestResourceSpec::ReadFileResult::INVALID_RESOURCE, {} },
{ "spec12.json", cmCTestResourceSpec::ReadFileResult::INVALID_ROOT, {} },
{ "spec13.json", cmCTestResourceSpec::ReadFileResult::JSON_PARSE_ERROR, {} },
{ "spec14.json", cmCTestResourceSpec::ReadFileResult::READ_OK, {} },
{ "spec15.json", cmCTestResourceSpec::ReadFileResult::READ_OK, {} },
{ "spec16.json", cmCTestResourceSpec::ReadFileResult::READ_OK, {} },
{ "spec17.json", cmCTestResourceSpec::ReadFileResult::INVALID_RESOURCE, {} },
{ "spec18.json", cmCTestResourceSpec::ReadFileResult::INVALID_RESOURCE, {} },
{ "spec19.json", cmCTestResourceSpec::ReadFileResult::INVALID_VERSION, {} },
{ "spec20.json", cmCTestResourceSpec::ReadFileResult::READ_OK, {} },
{ "spec21.json", cmCTestResourceSpec::ReadFileResult::INVALID_VERSION, {} },
{ "spec22.json", cmCTestResourceSpec::ReadFileResult::INVALID_VERSION, {} },
{ "spec23.json", cmCTestResourceSpec::ReadFileResult::INVALID_VERSION, {} },
{ "spec24.json", cmCTestResourceSpec::ReadFileResult::INVALID_VERSION, {} },
{ "spec25.json",
cmCTestResourceSpec::ReadFileResult::UNSUPPORTED_VERSION,
{} },
{ "spec26.json",
cmCTestResourceSpec::ReadFileResult::UNSUPPORTED_VERSION,
{} },
{ "spec27.json", cmCTestResourceSpec::ReadFileResult::INVALID_VERSION, {} },
{ "spec28.json", cmCTestResourceSpec::ReadFileResult::INVALID_VERSION, {} },
{ "spec29.json", cmCTestResourceSpec::ReadFileResult::INVALID_VERSION, {} },
{ "spec30.json", cmCTestResourceSpec::ReadFileResult::INVALID_VERSION, {} },
{ "spec31.json", cmCTestResourceSpec::ReadFileResult::INVALID_VERSION, {} },
{ "spec32.json", cmCTestResourceSpec::ReadFileResult::INVALID_VERSION, {} },
{ "spec33.json", cmCTestResourceSpec::ReadFileResult::INVALID_VERSION, {} },
{ "spec34.json", cmCTestResourceSpec::ReadFileResult::INVALID_VERSION, {} },
{ "spec35.json", cmCTestResourceSpec::ReadFileResult::INVALID_VERSION, {} },
{ "spec36.json", cmCTestResourceSpec::ReadFileResult::NO_VERSION, {} },
{ "noexist.json", cmCTestResourceSpec::ReadFileResult::FILE_NOT_FOUND, {} },
};
static bool testSpec(const std::string& path, bool expectedResult,
static bool testSpec(const std::string& path,
cmCTestResourceSpec::ReadFileResult expectedResult,
const cmCTestResourceSpec& expected)
{
cmCTestResourceSpec actual;
bool result = actual.ReadFromJSONFile(path);
auto result = actual.ReadFromJSONFile(path);
if (result != expectedResult) {
std::cout << "ReadFromJSONFile(\"" << path << "\") returned " << result
<< ", should be " << expectedResult << std::endl;
std::cout << "ReadFromJSONFile(\"" << path << "\") returned \""
<< cmCTestResourceSpec::ResultToString(result)
<< "\", should be \""
<< cmCTestResourceSpec::ResultToString(expectedResult) << "\""
<< std::endl;
return false;
}
if (result && actual != expected) {
if (actual != expected) {
std::cout << "ReadFromJSONFile(\"" << path
<< "\") did not give expected spec" << std::endl;
return false;

View File

@@ -285,7 +285,8 @@ static int doVerify(int argc, char const* const* argv)
std::set<std::string> testNameSet(testNameList.begin(), testNameList.end());
cmCTestResourceSpec spec;
if (!spec.ReadFromJSONFile(resFile)) {
if (spec.ReadFromJSONFile(resFile) !=
cmCTestResourceSpec::ReadFileResult::READ_OK) {
std::cout << "Could not read resource spec " << resFile << std::endl;
return 1;
}