diff --git a/src/core/domain/system.cpp b/src/core/domain/system.cpp index 6f29f97..bb95e16 100644 --- a/src/core/domain/system.cpp +++ b/src/core/domain/system.cpp @@ -21,14 +21,8 @@ System::~System() void System::Load(std::string module_name, std::string file_name, ImportType type) { + // Build the importer first, based on the import type. ImportBase *imp; - Module *mod = nullptr; - Parts *prts = nullptr; - - // Creation or retrieval of the module. - mod = mods->merge(module_name); - - // Parsing of the file based on the import type. if (type == ImportType::IMPORT_MENTOR) { imp = new ImportMentor(file_name); @@ -43,7 +37,17 @@ void System::Load(std::string module_name, std::string file_name, ImportType typ { throw std::runtime_error("Unknown import type"); } + + // Fail fast on a missing/unreadable file, before touching the module table, + // so a failed load never leaves behind an empty module. + if (!imp->is_open()) + { + delete imp; + throw std::runtime_error("cannot open file: " + file_name); + } + + // Creation or retrieval of the module, then parse into it. + Module *mod = mods->merge(module_name); imp->parse(mod->signals); - prts = imp->parts(); - mod->add(prts); + mod->add(imp->parts()); } \ No newline at end of file diff --git a/src/core/imports/import_base.hpp b/src/core/imports/import_base.hpp index b63a947..04a1c28 100644 --- a/src/core/imports/import_base.hpp +++ b/src/core/imports/import_base.hpp @@ -27,11 +27,22 @@ public: * * @param file_name Name of the file to be imported. */ - ImportBase(std::string file_name) : file_lines(std::fstream(file_name)) + ImportBase(std::string file_name) : file_lines(file_name, std::ios::in) { prts = new Parts(); }; + /** + * @brief Whether the input file was opened successfully. + * + * Opened read-only, so this is false only when the file is genuinely + * missing or unreadable (a read-only but present file still opens). + * System::Load checks it to fail fast instead of producing an empty module. + * + * @return true if the file stream is open. + */ + bool is_open() const { return file_lines.is_open(); } + /** * @brief Pure virtual method for parsing the file. * diff --git a/tests/test_load.cpp b/tests/test_load.cpp index db70413..38787dd 100644 --- a/tests/test_load.cpp +++ b/tests/test_load.cpp @@ -1,6 +1,7 @@ #include #include "core/app/load.hpp" +#include "core/domain/modules.hpp" #include "core/domain/system.hpp" #include @@ -50,15 +51,13 @@ TEST_CASE("load_module imports, drops singletons and reports counts") { std::remove(path); } -TEST_CASE("load_module on a missing file currently succeeds empty (no throw)") { - // Pre-existing behaviour, preserved by the extraction: ImportBase opens the - // stream without checking is_open(), so a missing file yields an empty - // module rather than an error. Pinned here so any future hardening of this - // path is a deliberate, visible change. +TEST_CASE("load_module fails cleanly on a missing file") { + // ImportBase opens read-only and System::Load checks is_open(), so a missing + // file is a clean error — and no empty module is left in the system. System sys; app::LoadResult r = app::load_module( &sys, "M", "/nonexistent-dir-xyz/nope.net", ImportType::IMPORT_MENTOR); - CHECK(r.ok); - CHECK(r.parts == 0); - CHECK(r.signals == 0); + CHECK_FALSE(r.ok); + CHECK(r.error.find("cannot open") != std::string::npos); + CHECK(sys.modules()->size() == 0); }