From a4599b3b9dcd2e09aa91f28c4251230d5de744d9 Mon Sep 17 00:00:00 2001 From: Raphael Maenle Date: Tue, 9 Jun 2026 14:32:09 +0200 Subject: [PATCH] config: multi-error YAML validator + --validate-config CLI flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The existing loader throws ConfigError on the first problem it hits. A customer with a tool-specific equipment.yaml that has six issues sees one, fixes, restarts, sees the next, fixes, restarts — six edit-restart cycles before the server even binds. Day-1 friction is the top support ticket source in fab integrations. This commit adds a parallel validator that does a separate read-only pass and surfaces *every* issue at once: $ secs_server --validate-config \ --config equipment.yaml \ --state-table control_state.yaml [error] equipment.yaml:5 svids[0].type — unknown SECS-II type `WTF` [error] equipment.yaml:7 alarms[0].category — value 200 out of range [0, 127] [error] equipment.yaml:9 host_commands[0].emit_ceid — CEID 999 not declared in `ceids` section 3 error(s), 0 warning(s) across 4 files What it catches: - Missing required fields (device.model_name, .software_rev, …) - Range violations (alarm category must be 0–127, spool streams 1–127, device.id fits u16, etc.) - Unknown enum values (SECS-II types, HCACK values, control/PJ/CJ state and event names — using the right case + snake convention the runtime parsers enforce) - Duplicate IDs within svids / dvids / ecids / ceids / alarms, duplicate PPIDs in recipes, duplicate command names in host_commands - Referential integrity: host_commands[*].emit_ceid must exist in ceids; host_commands[*].set_alarm must exist in alarms; emit_on_control_change must exist in ceids - PJ-table-specific: `NoState` sentinel rejected as `initial`, `from`, or `to` (matches loader's existing runtime check) - yaml-cpp Mark → 1-based line numbers when available What it doesn't catch (out of scope this round): - JSON Schema for editor red-squigglies (future) - Deep semantic checks across state-table reachability - ECID min/max value parsing (would need numeric type coupling) Tests cover: clean file passes; multi-error YAML surfaces every issue on a single pass; line numbers populate; control_state / process_job_state / control_job_state casing conventions; format_issues_to renders both severities; the shipped data/equipment.yaml etc. validate cleanly (regression tripwire if anyone breaks the demo configs). INTEGRATION.md §2.3 calls out the flag and suggests CI use. Co-Authored-By: Claude Opus 4.7 --- CMakeLists.txt | 2 + INTEGRATION.md | 25 +- apps/secs_server.cpp | 33 +- include/secsgem/config/validate.hpp | 57 +++ src/config/validate.cpp | 517 ++++++++++++++++++++++++++++ tests/test_config_validate.cpp | 204 +++++++++++ 6 files changed, 833 insertions(+), 5 deletions(-) create mode 100644 include/secsgem/config/validate.hpp create mode 100644 src/config/validate.cpp create mode 100644 tests/test_config_validate.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index d91d305..6ab8b50 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -64,6 +64,7 @@ add_library(secsgem src/gem/e84_state.cpp src/gem/host_handler.cpp src/config/loader.cpp + src/config/validate.cpp src/endpoint.cpp ) add_dependencies(secsgem generate_messages) @@ -142,6 +143,7 @@ add_executable(secsgem_tests tests/test_concurrency.cpp tests/test_thread_safety.cpp tests/test_persistence_upgrade.cpp + tests/test_config_validate.cpp ) target_link_libraries(secsgem_tests PRIVATE secsgem doctest::doctest) target_compile_definitions(secsgem_tests PRIVATE diff --git a/INTEGRATION.md b/INTEGRATION.md index 44d43e3..002eedf 100644 --- a/INTEGRATION.md +++ b/INTEGRATION.md @@ -129,7 +129,30 @@ io.run(); the start. The repo's `apps/secs_server.cpp` is a complete worked example — copy it verbatim, then customize the YAML to your tool. -### 2.3. Run it +### 2.3. Validate before you run + +YAML edits are easy to get wrong: an unknown SECS-II type, a +duplicate ID, a `host_command` referencing a CEID you forgot to +declare. The server has a `--validate-config` mode that reads every +YAML, accumulates *every* problem it can find, prints them with file +and line number, and exits 0 / 1 without binding the port: + +```sh +secs_server --validate-config \ + --config /etc/acme/equipment.yaml \ + --state-table /etc/acme/control_state.yaml \ + --pj-state-table /etc/acme/process_job_state.yaml \ + --cj-state-table /etc/acme/control_job_state.yaml +# [error] equipment.yaml:5 svids[0].type — unknown SECS-II type `WTF` +# [error] equipment.yaml:7 alarms[0].category — value 200 out of range [0, 127] +# [error] equipment.yaml:9 host_commands[0].emit_ceid — CEID 999 not declared in `ceids` section +# 3 error(s), 0 warning(s) across 4 files +``` + +Run this in CI on every config change and you skip the slow +load-fail-edit-restart loop the first deployment otherwise becomes. + +### 2.4. Run it ```sh docker compose up server # or your own deployment diff --git a/apps/secs_server.cpp b/apps/secs_server.cpp index c6d3a6d..8a9c791 100644 --- a/apps/secs_server.cpp +++ b/apps/secs_server.cpp @@ -13,6 +13,7 @@ #include #include "secsgem/config/loader.hpp" +#include "secsgem/config/validate.hpp" #include "secsgem/endpoint.hpp" #include "secsgem/gem/control_state.hpp" #include "secsgem/gem/data_model.hpp" @@ -35,6 +36,12 @@ std::string arg(int argc, char** argv, const std::string& key, const std::string return def; } +bool has_flag(int argc, char** argv, const std::string& key) { + for (int i = 1; i < argc; ++i) + if (key == argv[i]) return true; + return false; +} + constexpr uint32_t kSvidControlState = 1; constexpr uint32_t kSvidClock = 2; @@ -49,10 +56,30 @@ int main(int argc, char** argv) { const auto port = static_cast(std::stoi(arg(argc, argv, "--port", "5000"))); const auto equipment_yaml = arg(argc, argv, "--config", "/app/data/equipment.yaml"); const auto state_yaml = arg(argc, argv, "--state-table", "/app/data/control_state.yaml"); + const auto pj_state_yaml = arg(argc, argv, "--pj-state-table", + "/app/data/process_job_state.yaml"); + const auto cj_state_yaml = arg(argc, argv, "--cj-state-table", + "/app/data/control_job_state.yaml"); const auto spool_dir = arg(argc, argv, "--spool-dir", ""); + const bool validate_only = has_flag(argc, argv, "--validate-config"); auto logfn = [](const std::string& m) { std::cout << "[equip] " << m << std::endl; }; + // --validate-config: read every YAML, accumulate every issue we can + // find, print to stderr, and exit 0/1. Does NOT bind the port — this + // is the day-1 friction killer the README points customers at. + if (validate_only) { + config::ConfigValidator v; + v.validate_equipment(equipment_yaml); + v.validate_control_state(state_yaml); + v.validate_process_job_state(pj_state_yaml); + v.validate_control_job_state(cj_state_yaml); + config::format_issues_to(std::cerr, v.issues()); + std::cerr << v.error_count() << " error(s), " + << v.warning_count() << " warning(s) across 4 files\n"; + return v.has_errors() ? 1 : 0; + } + auto model = std::make_shared(); if (!spool_dir.empty()) { model->spool.enable_persistence(spool_dir); @@ -150,10 +177,8 @@ int main(int argc, char** argv) { }); // ---- E40/E94: load the PJ/CJ transition tables and wire emitters ----- - const auto pj_state_yaml = arg(argc, argv, "--pj-state-table", - "/app/data/process_job_state.yaml"); - const auto cj_state_yaml = arg(argc, argv, "--cj-state-table", - "/app/data/control_job_state.yaml"); + // (pj_state_yaml / cj_state_yaml already parsed at the top so + // --validate-config sees them.) config::ProcessJobStateConfig pj_cfg; config::ControlJobStateConfig cj_cfg; try { diff --git a/include/secsgem/config/validate.hpp b/include/secsgem/config/validate.hpp new file mode 100644 index 0000000..01b4f69 --- /dev/null +++ b/include/secsgem/config/validate.hpp @@ -0,0 +1,57 @@ +#pragma once + +#include +#include +#include +#include +#include + +// Multi-error config validator. +// +// The existing `config::load_*` functions throw `ConfigError` on the +// first problem they encounter — that's fine for runtime but terrible +// for customers iterating on a new tool's YAML. This validator does a +// separate read-only pass that collects every issue it can find, so a +// `--validate-config` invocation surfaces the whole list in one shot. +// +// Each issue carries the file, line (1-based, from yaml-cpp's Mark when +// available), a structured field path (e.g. "svids[2].type"), and a +// human message. Stream the issues to stderr with +// `format_issues_to(os, issues)` for the canonical presentation. +namespace secsgem::config { + +struct ConfigIssue { + enum class Severity : uint8_t { Error = 0, Warning = 1 }; + + std::string file; + std::optional line; // 1-based; nullopt when unknown + std::string field_path; // e.g. "host_commands[3].emit_ceid" + std::string message; + Severity severity = Severity::Error; +}; + +class ConfigValidator { + public: + // Each validate_* method reads the given YAML and appends issues. + // Returns false if any new errors were emitted by this call (warnings + // do not flip the return value). + bool validate_equipment(const std::string& yaml_path); + bool validate_control_state(const std::string& yaml_path); + bool validate_process_job_state(const std::string& yaml_path); + bool validate_control_job_state(const std::string& yaml_path); + + const std::vector& issues() const { return issues_; } + bool has_errors() const; + std::size_t error_count() const; + std::size_t warning_count() const; + + private: + std::vector issues_; +}; + +// Pretty-prints every issue, one per line, in a format intended for +// stderr. Errors are tagged `[error]`, warnings `[warn]`. +void format_issues_to(std::ostream& os, + const std::vector& issues); + +} // namespace secsgem::config diff --git a/src/config/validate.cpp b/src/config/validate.cpp new file mode 100644 index 0000000..302d911 --- /dev/null +++ b/src/config/validate.cpp @@ -0,0 +1,517 @@ +#include "secsgem/config/validate.hpp" + +#include + +#include +#include +#include +#include +#include +#include +#include + +#include "secsgem/gem/control_job_state.hpp" +#include "secsgem/gem/control_state.hpp" +#include "secsgem/gem/process_job_state.hpp" + +namespace secsgem::config { + +namespace { + +namespace gem = secsgem::gem; + +// Map yaml-cpp's Mark to a 1-based line number when available. +std::optional line_of(const YAML::Node& n) { + const auto m = n.Mark(); + if (m.line < 0) return std::nullopt; + return m.line + 1; // yaml-cpp marks are 0-based +} + +class Sink { + public: + Sink(std::vector& out, std::string file) + : out_(out), file_(std::move(file)) {} + + void error(const std::string& field_path, const std::string& msg, + const YAML::Node& at = {}) { + out_.push_back({file_, line_of(at), field_path, msg, + ConfigIssue::Severity::Error}); + } + + void warn(const std::string& field_path, const std::string& msg, + const YAML::Node& at = {}) { + out_.push_back({file_, line_of(at), field_path, msg, + ConfigIssue::Severity::Warning}); + } + + private: + std::vector& out_; + std::string file_; +}; + +bool valid_secs_type(const std::string& t) { + static const std::set ok = { + "ASCII", "BOOLEAN", "BINARY", + "U1", "U2", "U4", "U8", + "I1", "I2", "I4", "I8", + "F4", "F8", + }; + return ok.count(t) > 0; +} + +bool valid_hcack(const std::string& s) { + static const std::set ok = { + "Accept", "InvalidCommand", "CannotDoNow", "ParameterInvalid", + "AcceptedWillFinishLater", "Rejected", "InvalidObject", + }; + return ok.count(s) > 0; +} + +// Range-checked integer extraction. Returns nullopt and emits an +// error if the node is missing, non-scalar, or out of range. +template +std::optional as_int_in_range(const YAML::Node& n, Sink& sink, + const std::string& path, + int64_t min, int64_t max) { + if (!n) { + sink.error(path, "missing required integer"); + return std::nullopt; + } + if (!n.IsScalar()) { + sink.error(path, "expected an integer", n); + return std::nullopt; + } + int64_t v = 0; + try { + v = n.as(); + } catch (const YAML::Exception&) { + sink.error(path, "value `" + n.Scalar() + "` is not a valid integer", n); + return std::nullopt; + } + if (v < min || v > max) { + sink.error(path, "value " + std::to_string(v) + " out of range [" + + std::to_string(min) + ", " + std::to_string(max) + + "]", + n); + return std::nullopt; + } + return static_cast(v); +} + +std::optional as_nonempty_string(const YAML::Node& n, + Sink& sink, + const std::string& path) { + if (!n) { + sink.error(path, "missing required string"); + return std::nullopt; + } + if (!n.IsScalar()) { + sink.error(path, "expected a string", n); + return std::nullopt; + } + auto s = n.as(); + if (s.empty()) { + sink.error(path, "string must not be empty", n); + return std::nullopt; + } + return s; +} + +// Validate one entry that has the same (id, name, units, type, value) +// shape as SVID/DVID/ECID. Returns the parsed id on success so the +// caller can dedupe. +std::optional validate_typed_entry( + const YAML::Node& entry, Sink& sink, const std::string& path) { + auto id = as_int_in_range(entry["id"], sink, path + ".id", + 0, UINT32_MAX); + as_nonempty_string(entry["name"], sink, path + ".name"); + if (auto tn = entry["type"]) { + auto t = tn.as(); + if (!valid_secs_type(t)) { + sink.error(path + ".type", + "unknown SECS-II type `" + t + + "` (expected one of ASCII/BOOLEAN/BINARY/U1-U8/I1-I8/F4/F8)", + tn); + } + } else { + sink.error(path + ".type", "missing required field `type`"); + } + return id; +} + +void validate_equipment_block(YAML::Node& root, Sink& sink) { + std::set ceids; + std::set alarm_ids; + + if (!root["device"]) { + sink.error("device", "missing required `device` block"); + } else { + auto dev = root["device"]; + as_int_in_range(dev["id"], sink, "device.id", 0, UINT16_MAX); + as_nonempty_string(dev["model_name"], sink, "device.model_name"); + as_nonempty_string(dev["software_rev"], sink, "device.software_rev"); + } + + if (auto caps = root["capabilities"]; caps && caps.IsSequence()) { + for (std::size_t i = 0; i < caps.size(); ++i) { + const auto path = "capabilities[" + std::to_string(i) + "]"; + const auto& c = caps[i]; + as_int_in_range(c["code"], sink, path + ".code", 0, 255); + as_nonempty_string(c["name"], sink, path + ".name"); + } + } + + auto validate_entry_list = [&](const YAML::Node& seq, const char* key, + std::set* dedupe = nullptr) { + if (!seq) return; + if (!seq.IsSequence()) { + sink.error(key, "expected a sequence", seq); + return; + } + std::set local; + for (std::size_t i = 0; i < seq.size(); ++i) { + const auto path = std::string(key) + "[" + std::to_string(i) + "]"; + auto id = validate_typed_entry(seq[i], sink, path); + if (id) { + if (!local.insert(*id).second) { + sink.error(path + ".id", + "duplicate id " + std::to_string(*id) + " in " + key, + seq[i]); + } + if (dedupe) dedupe->insert(*id); + } + } + }; + validate_entry_list(root["svids"], "svids"); + validate_entry_list(root["dvids"], "dvids"); + validate_entry_list(root["ecids"], "ecids"); + + // CEIDs have a simpler shape (id + name). + if (auto ces = root["ceids"]) { + if (!ces.IsSequence()) { + sink.error("ceids", "expected a sequence", ces); + } else { + for (std::size_t i = 0; i < ces.size(); ++i) { + const auto path = "ceids[" + std::to_string(i) + "]"; + auto id = as_int_in_range(ces[i]["id"], sink, path + ".id", + 0, UINT32_MAX); + as_nonempty_string(ces[i]["name"], sink, path + ".name"); + if (id && !ceids.insert(*id).second) { + sink.error(path + ".id", + "duplicate CEID " + std::to_string(*id), + ces[i]); + } + } + } + } + + if (auto als = root["alarms"]) { + if (!als.IsSequence()) { + sink.error("alarms", "expected a sequence", als); + } else { + for (std::size_t i = 0; i < als.size(); ++i) { + const auto path = "alarms[" + std::to_string(i) + "]"; + auto id = as_int_in_range(als[i]["id"], sink, path + ".id", + 0, UINT32_MAX); + as_nonempty_string(als[i]["text"], sink, path + ".text"); + // ALCD lower-7 bits are the category bitmap (E5 §10.3). Bit 7 + // is the set/clear flag, set at emit time — must not be in YAML. + as_int_in_range(als[i]["category"], sink, + path + ".category", 0, 127); + if (id && !alarm_ids.insert(*id).second) { + sink.error(path + ".id", + "duplicate ALID " + std::to_string(*id), als[i]); + } + } + } + } + + if (auto recs = root["recipes"]) { + if (!recs.IsSequence()) { + sink.error("recipes", "expected a sequence", recs); + } else { + std::set seen; + for (std::size_t i = 0; i < recs.size(); ++i) { + const auto path = "recipes[" + std::to_string(i) + "]"; + auto id = as_nonempty_string(recs[i]["id"], sink, path + ".id"); + if (!recs[i]["body"]) { + sink.error(path + ".body", "missing required `body`"); + } + if (id && !seen.insert(*id).second) { + sink.error(path + ".id", "duplicate PPID `" + *id + "`", recs[i]); + } + } + } + } + + if (auto cmds = root["host_commands"]) { + if (!cmds.IsSequence()) { + sink.error("host_commands", "expected a sequence", cmds); + } else { + std::set names; + for (std::size_t i = 0; i < cmds.size(); ++i) { + const auto path = "host_commands[" + std::to_string(i) + "]"; + auto name = as_nonempty_string(cmds[i]["name"], sink, path + ".name"); + if (auto ack = cmds[i]["ack"]) { + auto s = ack.as(); + if (!valid_hcack(s)) + sink.error(path + ".ack", "unknown HCACK `" + s + "`", ack); + } else { + sink.error(path + ".ack", "missing required `ack`"); + } + if (auto e = cmds[i]["emit_ceid"]) { + auto v = as_int_in_range(e, sink, path + ".emit_ceid", + 0, UINT32_MAX); + if (v && !ceids.count(*v)) { + sink.error(path + ".emit_ceid", + "CEID " + std::to_string(*v) + + " not declared in `ceids` section", + e); + } + } + if (auto a = cmds[i]["set_alarm"]) { + auto v = as_int_in_range(a, sink, path + ".set_alarm", + 0, UINT32_MAX); + if (v && !alarm_ids.count(*v)) { + sink.error(path + ".set_alarm", + "ALID " + std::to_string(*v) + + " not declared in `alarms` section", + a); + } + } + if (auto f = cmds[i]["force_spool"]) { + if (!f.IsScalar()) sink.error(path + ".force_spool", + "expected a boolean", f); + } + if (name && !names.insert(*name).second) { + sink.error(path + ".name", "duplicate command name `" + *name + "`", + cmds[i]); + } + } + } + } + + if (auto sp = root["spool"]) { + if (auto s = sp["spoolable_streams"]) { + if (!s.IsSequence()) { + sink.error("spool.spoolable_streams", "expected a sequence", s); + } else { + for (std::size_t i = 0; i < s.size(); ++i) { + // SECS-II streams are 1..127 (top bit is the W flag). + as_int_in_range( + s[i], sink, + "spool.spoolable_streams[" + std::to_string(i) + "]", + 1, 127); + } + } + } + if (auto m = sp["max_size"]) { + as_int_in_range(m, sink, "spool.max_size", 1, INT32_MAX); + } + } + + if (auto e = root["emit_on_control_change"]) { + if (!e.IsNull()) { + auto v = as_int_in_range(e, sink, "emit_on_control_change", + 0, UINT32_MAX); + if (v && !ceids.count(*v)) { + sink.error("emit_on_control_change", + "CEID " + std::to_string(*v) + + " not declared in `ceids` section", + e); + } + } + } +} + +YAML::Node try_load(const std::string& path, Sink& sink) { + try { + return YAML::LoadFile(path); + } catch (const YAML::Exception& e) { + sink.error("", std::string("YAML parse error: ") + e.what()); + return YAML::Node{}; + } +} + +} // namespace + +bool ConfigValidator::has_errors() const { return error_count() > 0; } + +std::size_t ConfigValidator::error_count() const { + return static_cast(std::count_if( + issues_.begin(), issues_.end(), + [](const auto& i) { return i.severity == ConfigIssue::Severity::Error; })); +} + +std::size_t ConfigValidator::warning_count() const { + return issues_.size() - error_count(); +} + +bool ConfigValidator::validate_equipment(const std::string& path) { + const auto baseline = error_count(); + Sink sink(issues_, path); + YAML::Node root = try_load(path, sink); + if (root) validate_equipment_block(root, sink); + return error_count() == baseline; +} + +bool ConfigValidator::validate_control_state(const std::string& path) { + const auto baseline = error_count(); + Sink sink(issues_, path); + YAML::Node root = try_load(path, sink); + if (!root) return false; + + if (auto in = root["initial"]) { + if (!gem::parse_control_state(in.as())) { + sink.error("initial", "unknown control state `" + in.as() + "`", + in); + } + } + + const auto ts = root["transitions"]; + if (!ts || !ts.IsSequence()) { + sink.error("transitions", "missing or non-sequence"); + return false; + } + for (std::size_t i = 0; i < ts.size(); ++i) { + const auto path_i = "transitions[" + std::to_string(i) + "]"; + const auto& row = ts[i]; + auto from_n = row["from"]; + auto on_n = row["on"]; + if (!from_n) sink.error(path_i + ".from", "missing required `from`"); + if (!on_n) sink.error(path_i + ".on", "missing required `on`"); + if (from_n && !gem::parse_control_state(from_n.as())) { + sink.error(path_i + ".from", + "unknown control state `" + from_n.as() + "`", + from_n); + } + if (on_n && !gem::parse_control_event(on_n.as())) { + sink.error(path_i + ".on", + "unknown control event `" + on_n.as() + "`", + on_n); + } + if (auto to = row["to"]) { + if (!gem::parse_control_state(to.as())) { + sink.error(path_i + ".to", + "unknown control state `" + to.as() + "`", + to); + } + } + if (auto th = row["then"]) { + if (!gem::parse_control_state(th.as())) { + sink.error(path_i + ".then", + "unknown control state `" + th.as() + "`", + th); + } + } + } + return error_count() == baseline; +} + +bool ConfigValidator::validate_process_job_state(const std::string& path) { + const auto baseline = error_count(); + Sink sink(issues_, path); + YAML::Node root = try_load(path, sink); + if (!root) return false; + + if (auto in = root["initial"]) { + auto parsed = gem::parse_process_job_state(in.as()); + if (!parsed) { + sink.error("initial", + "unknown PJ state `" + in.as() + "`", in); + } else if (*parsed == gem::ProcessJobState::NoState) { + sink.error("initial", + "`NoState` is a sentinel and cannot be the initial state", + in); + } + } + + const auto ts = root["transitions"]; + if (!ts || !ts.IsSequence()) { + sink.error("transitions", "missing or non-sequence"); + return false; + } + for (std::size_t i = 0; i < ts.size(); ++i) { + const auto path_i = "transitions[" + std::to_string(i) + "]"; + const auto& row = ts[i]; + if (auto from = row["from"]) { + auto parsed = gem::parse_process_job_state(from.as()); + if (!parsed) + sink.error(path_i + ".from", "unknown PJ state", from); + else if (*parsed == gem::ProcessJobState::NoState) + sink.error(path_i + ".from", + "`NoState` is a sentinel and cannot appear as `from`", from); + } else { + sink.error(path_i + ".from", "missing required `from`"); + } + if (auto on = row["on"]) { + if (!gem::parse_process_job_event(on.as())) + sink.error(path_i + ".on", "unknown PJ event", on); + } else { + sink.error(path_i + ".on", "missing required `on`"); + } + if (auto to = row["to"]) { + auto parsed = gem::parse_process_job_state(to.as()); + if (!parsed) + sink.error(path_i + ".to", "unknown PJ state", to); + else if (*parsed == gem::ProcessJobState::NoState) + sink.error(path_i + ".to", + "`NoState` is a sentinel and cannot appear as `to`", to); + } + } + return error_count() == baseline; +} + +bool ConfigValidator::validate_control_job_state(const std::string& path) { + const auto baseline = error_count(); + Sink sink(issues_, path); + YAML::Node root = try_load(path, sink); + if (!root) return false; + + if (auto in = root["initial"]) { + if (!gem::parse_control_job_state(in.as())) + sink.error("initial", + "unknown CJ state `" + in.as() + "`", in); + } + + const auto ts = root["transitions"]; + if (!ts || !ts.IsSequence()) { + sink.error("transitions", "missing or non-sequence"); + return false; + } + for (std::size_t i = 0; i < ts.size(); ++i) { + const auto path_i = "transitions[" + std::to_string(i) + "]"; + const auto& row = ts[i]; + if (auto from = row["from"]) { + if (!gem::parse_control_job_state(from.as())) + sink.error(path_i + ".from", "unknown CJ state", from); + } else { + sink.error(path_i + ".from", "missing required `from`"); + } + if (auto on = row["on"]) { + if (!gem::parse_control_job_event(on.as())) + sink.error(path_i + ".on", "unknown CJ event", on); + } else { + sink.error(path_i + ".on", "missing required `on`"); + } + if (auto to = row["to"]) { + if (!gem::parse_control_job_state(to.as())) + sink.error(path_i + ".to", "unknown CJ state", to); + } + } + return error_count() == baseline; +} + +void format_issues_to(std::ostream& os, + const std::vector& issues) { + for (const auto& i : issues) { + os << (i.severity == ConfigIssue::Severity::Error ? "[error] " + : "[warn] ") + << i.file; + if (i.line) os << ":" << *i.line; + if (!i.field_path.empty()) os << " " << i.field_path; + os << " — " << i.message << "\n"; + } +} + +} // namespace secsgem::config diff --git a/tests/test_config_validate.cpp b/tests/test_config_validate.cpp new file mode 100644 index 0000000..a57b798 --- /dev/null +++ b/tests/test_config_validate.cpp @@ -0,0 +1,204 @@ +// Multi-error config validator tests. Crafts intentionally broken +// YAML, runs the validator, and asserts that the issues vector +// surfaces every problem on a single pass — that's the contract the +// --validate-config CLI flag relies on. + +#include + +#include +#include +#include +#include +#include +#include + +#include "secsgem/config/validate.hpp" + +namespace fs = std::filesystem; +using secsgem::config::ConfigIssue; +using secsgem::config::ConfigValidator; + +namespace { + +fs::path scratch_path(const char* tag, const char* ext) { + std::random_device rd; + auto dir = fs::temp_directory_path() / + (std::string("secsgem-cfg-") + tag + "-" + std::to_string(rd())); + fs::create_directories(dir); + return dir / (std::string("file.") + ext); +} + +void write(const fs::path& p, const std::string& contents) { + std::ofstream out(p); + out << contents; +} + +bool any_match(const std::vector& issues, + const std::string& field_substr) { + return std::any_of(issues.begin(), issues.end(), [&](const ConfigIssue& i) { + return i.field_path.find(field_substr) != std::string::npos; + }); +} + +} // namespace + +TEST_CASE("Validate: equipment.yaml — clean file produces zero errors") { + auto p = scratch_path("clean", "yaml"); + write(p, R"YAML( +device: + id: 0 + model_name: "TEST" + software_rev: "0.1" +ceids: + - {id: 300, name: ProcessStarted} +alarms: + - {id: 1, text: "high temp", category: 4} +recipes: + - {id: "R-1", body: "STEP HEAT"} +host_commands: + - {name: START, ack: Accept, emit_ceid: 300, set_alarm: 1} +)YAML"); + ConfigValidator v; + CHECK(v.validate_equipment(p.string())); + CHECK(v.error_count() == 0); +} + +TEST_CASE("Validate: equipment.yaml — multiple errors surface in one pass") { + auto p = scratch_path("multi", "yaml"); + write(p, R"YAML( +device: + id: 0 + model_name: "TEST" + # software_rev: missing +svids: + - {id: 1, name: ControlState, type: ASCII, value: ""} + - {id: 1, name: Duplicate, type: U4, value: 0} +ecids: + - {id: 10, name: BadType, type: WTF, value: 1} +alarms: + - {id: 1, text: "x", category: 200} # out of range (max 127) +host_commands: + - {name: START, ack: NotARealAck, emit_ceid: 999, set_alarm: 999} +emit_on_control_change: 12345 +)YAML"); + ConfigValidator v; + CHECK_FALSE(v.validate_equipment(p.string())); + const auto& issues = v.issues(); + // We expect at least: missing software_rev, duplicate SVID id, + // unknown type WTF, alarm category out of range, unknown HCACK, + // CEID 999 not declared, ALID 999 not declared, emit_on_control_change CEID undeclared. + CHECK(v.error_count() >= 8); + CHECK(any_match(issues, "device.software_rev")); + CHECK(any_match(issues, "svids[1].id")); + CHECK(any_match(issues, "ecids[0].type")); + CHECK(any_match(issues, "alarms[0].category")); + CHECK(any_match(issues, "host_commands[0].ack")); + CHECK(any_match(issues, "host_commands[0].emit_ceid")); + CHECK(any_match(issues, "host_commands[0].set_alarm")); + CHECK(any_match(issues, "emit_on_control_change")); +} + +TEST_CASE("Validate: equipment.yaml — line numbers populate when known") { + auto p = scratch_path("lines", "yaml"); + write(p, R"YAML( +device: + id: 0 + model_name: "TEST" + software_rev: "0.1" +alarms: + - {id: 1, text: "ok", category: 4} + - {id: 1, text: "dup", category: 4} +)YAML"); + ConfigValidator v; + v.validate_equipment(p.string()); + bool found_lineno = false; + for (const auto& i : v.issues()) { + if (i.field_path.find("alarms[1]") != std::string::npos && i.line) { + found_lineno = true; + // line 8 in our heredoc (the second alarm). + CHECK(*i.line >= 7); + CHECK(*i.line <= 9); + } + } + CHECK(found_lineno); +} + +TEST_CASE("Validate: control_state.yaml — unknown state name") { + auto p = scratch_path("ctrl", "yaml"); + write(p, R"YAML( +initial: HostOffline +transitions: + - {from: NotAState, on: host_request_online, to: OnlineRemote, ack: Accept} + - {from: HostOffline, on: not_an_event, to: HostOffline} +)YAML"); + ConfigValidator v; + CHECK_FALSE(v.validate_control_state(p.string())); + const auto& issues = v.issues(); + CHECK(any_match(issues, "transitions[0].from")); + CHECK(any_match(issues, "transitions[1].on")); +} + +TEST_CASE("Validate: process_job_state.yaml — NoState sentinel rejected") { + auto p = scratch_path("pj", "yaml"); + write(p, R"YAML( +initial: NoState +transitions: + - {from: NoState, on: select, to: SettingUp} + - {from: Queued, on: select, to: NoState} +)YAML"); + ConfigValidator v; + CHECK_FALSE(v.validate_process_job_state(p.string())); + // All three NoState placements should each surface as an error. + std::size_t no_state_hits = 0; + for (const auto& i : v.issues()) { + if (i.message.find("NoState") != std::string::npos) ++no_state_hits; + } + CHECK(no_state_hits == 3); +} + +TEST_CASE("Validate: control_job_state.yaml — clean file passes") { + auto p = scratch_path("cj-clean", "yaml"); + // State names are CamelCase, event names are snake_case — the + // convention the runtime parsers in src/gem/control_job_state.cpp + // enforce. + write(p, R"YAML( +initial: Queued +transitions: + - {from: Queued, on: select, to: Selected, ack: Accept} + - {from: Selected, on: start, to: Executing} +)YAML"); + ConfigValidator v; + CHECK(v.validate_control_job_state(p.string())); + CHECK(v.error_count() == 0); +} + +TEST_CASE("Validate: format_issues_to renders error and warning rows") { + std::vector issues = { + {"a.yaml", 12, "device.id", "out of range", + ConfigIssue::Severity::Error}, + {"b.yaml", std::nullopt, "spool.max_size", "consider raising", + ConfigIssue::Severity::Warning}, + }; + std::ostringstream os; + secsgem::config::format_issues_to(os, issues); + const auto out = os.str(); + CHECK(out.find("[error] a.yaml:12 device.id") != std::string::npos); + CHECK(out.find("[warn] b.yaml spool.max_size") != std::string::npos); +} + +TEST_CASE("Validate: ships data/equipment.yaml without errors") { + // The shipped demo config is the canonical "good" sample; if anyone + // edits data/equipment.yaml in a way that breaks the validator's + // invariants, this test fails loudly. + ConfigValidator v; + v.validate_equipment(SECSGEM_DATA_DIR "/equipment.yaml"); + v.validate_control_state(SECSGEM_DATA_DIR "/control_state.yaml"); + v.validate_process_job_state(SECSGEM_DATA_DIR "/process_job_state.yaml"); + v.validate_control_job_state(SECSGEM_DATA_DIR "/control_job_state.yaml"); + if (v.has_errors()) { + std::ostringstream os; + secsgem::config::format_issues_to(os, v.issues()); + FAIL(os.str()); + } + CHECK(v.error_count() == 0); +}