Skip to content

Commit a547325

Browse files
Saumya-RSaumya-R
committed
review fixes for error code
1 parent f276861 commit a547325

File tree

4 files changed

+63
-27
lines changed

4 files changed

+63
-27
lines changed

‎tests/cpp_test_scenarios/src/cit/test_default_values.cpp‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
#include <vector>
2323

2424
// Helper to print info logs in a Python-parsable way
25+
26+
constexpr double kFloatEpsilon = 1e-6;
2527
const std::string kTargetName{"cpp_test_scenarios::cit::default_values"};
2628
using score::mw::per::kvs::KvsValue;
2729

@@ -95,7 +97,7 @@ void DefaultValuesScenario::run(const std::optional<std::string> &input) const {
9597
} else if (std::abs(
9698
std::get<double>(get_default_result.value().getValue()) -
9799
std::get<double>(get_value_result.value().getValue())) <
98-
1e-6) {
100+
kFloatEpsilon) {
99101
value_is_default = "Ok(true)";
100102
} else {
101103
value_is_default = "Ok(false)";
@@ -133,7 +135,7 @@ void DefaultValuesScenario::run(const std::optional<std::string> &input) const {
133135
try {
134136
double v = std::get<double>(cur_val->getValue());
135137
double d = std::get<double>(def_val->getValue());
136-
if (v == d)
138+
if (std::abs(v - d) < kFloatEpsilon)
137139
value_is_default = "Ok(true)";
138140
} catch (const std::bad_variant_access &e) {
139141
throw;
@@ -216,7 +218,8 @@ void RemoveKeyScenario::run(const std::optional<std::string> &input) const {
216218
current_value == "Err(KeyNotFound)") {
217219
value_is_default = "Err(KeyNotFound)";
218220
} else if (std::abs(std::get<double>(get_default->getValue()) -
219-
std::get<double>(get_value->getValue())) < 1e-6) {
221+
std::get<double>(get_value->getValue())) <
222+
kFloatEpsilon) {
220223
value_is_default = "Ok(true)";
221224
} else {
222225
value_is_default = "Ok(false)";
@@ -234,7 +237,7 @@ void RemoveKeyScenario::run(const std::optional<std::string> &input) const {
234237
get_value->getType() == KvsValue::Type::f64) {
235238
double v = std::get<double>(get_value->getValue());
236239
double d = std::get<double>(get_default->getValue());
237-
if (v == d)
240+
if (std::abs(v - d) < kFloatEpsilon)
238241
value_is_default = "Ok(true)";
239242
}
240243
// Format current_value for log
@@ -265,7 +268,7 @@ void RemoveKeyScenario::run(const std::optional<std::string> &input) const {
265268
get_value->getType() == KvsValue::Type::f64) {
266269
double v = std::get<double>(get_value->getValue());
267270
double d = std::get<double>(get_default->getValue());
268-
if (v == d)
271+
if (std::abs(v - d) < kFloatEpsilon)
269272
value_is_default = "Ok(true)";
270273
}
271274
}
@@ -303,7 +306,8 @@ void ResetAllKeysScenario::run(const std::optional<std::string> &input) const {
303306
get_value->getType() == KvsValue::Type::f64) {
304307
double v = std::get<double>(get_value->getValue());
305308
double d = std::get<double>(get_default->getValue());
306-
value_is_default = (v == d) ? "Ok(true)" : "Ok(false)";
309+
value_is_default =
310+
(std::abs(v - d) < kFloatEpsilon) ? "Ok(true)" : "Ok(false)";
307311
} else if (!get_default) {
308312
value_is_default = "Err(KeyNotFound)";
309313
} else {
@@ -350,7 +354,8 @@ void ResetAllKeysScenario::run(const std::optional<std::string> &input) const {
350354
get_value->getType() == KvsValue::Type::f64) {
351355
double v = std::get<double>(get_value->getValue());
352356
double d = std::get<double>(get_default->getValue());
353-
value_is_default = (v == d) ? "Ok(true)" : "Ok(false)";
357+
value_is_default =
358+
(std::abs(v - d) < kFloatEpsilon) ? "Ok(true)" : "Ok(false)";
354359
} else if (!get_default) {
355360
value_is_default = "Err(KeyNotFound)";
356361
} else {
@@ -395,7 +400,8 @@ void ResetSingleKeyScenario::run(
395400
get_value->getType() == KvsValue::Type::f64) {
396401
double v = std::get<double>(get_value->getValue());
397402
double d = std::get<double>(get_default->getValue());
398-
value_is_default = (v == d) ? "Ok(true)" : "Ok(false)";
403+
value_is_default =
404+
(std::abs(v - d) < kFloatEpsilon) ? "Ok(true)" : "Ok(false)";
399405
} else if (!get_default) {
400406
value_is_default = "Err(KeyNotFound)";
401407
} else {
@@ -452,7 +458,8 @@ void ResetSingleKeyScenario::run(
452458
get_value->getType() == KvsValue::Type::f64) {
453459
double v = std::get<double>(get_value->getValue());
454460
double d = std::get<double>(get_default->getValue());
455-
value_is_default = (v == d) ? "Ok(true)" : "Ok(false)";
461+
value_is_default =
462+
(std::abs(v - d) < kFloatEpsilon) ? "Ok(true)" : "Ok(false)";
456463
std::ostringstream oss;
457464
oss.precision(1);
458465
oss << std::fixed << v;

‎tests/cpp_test_scenarios/src/helpers/kvs_instance.hpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@ static score::mw::per::kvs::Kvs kvs_instance(const KvsParameters &params) {
1818
}
1919
auto kvs_ptr = builder.build();
2020
if (!kvs_ptr) {
21-
throw std::runtime_error{
22-
"KVS creation failed: build() returned null (possible file not found, "
23-
"JSON parse error, or corruption)"};
21+
throw ScenarioError(score::mw::per::kvs::ErrorCode::JsonParserError,
22+
"KVS creation failed: build() returned null (possible "
23+
"file not found, JSON parse error, or corruption)");
2424
}
2525
return std::move(*kvs_ptr);
2626
}

‎tests/cpp_test_scenarios/src/helpers/kvs_parameters.hpp‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,13 @@
1+
#include "internal/error.hpp"
2+
#include <stdexcept>
3+
4+
// Custom exception type for error code propagation (shared with main.cpp)
5+
class ScenarioError : public std::runtime_error {
6+
public:
7+
score::mw::per::kvs::ErrorCode code;
8+
ScenarioError(score::mw::per::kvs::ErrorCode code, const std::string &msg)
9+
: std::runtime_error(msg), code(code) {}
10+
};
111
#pragma once
212
#include <cstdint>
313
#include <optional>
@@ -25,22 +35,28 @@ KvsParameters map_to_params(const std::string &data) {
2535
JsonParser parser;
2636
auto any_res{parser.FromBuffer(data)};
2737
if (!any_res) {
28-
throw std::runtime_error{"Failed to parse JSON data"};
38+
throw ScenarioError(score::mw::per::kvs::ErrorCode::JsonParserError,
39+
"Failed to parse JSON data");
2940
}
3041
const auto &map_root{
3142
any_res.value().As<Object>().value().get().at("kvs_parameters")};
3243
const auto &obj_root{map_root.As<Object>().value().get()};
3344

3445
KvsParameters params;
3546
params.instance_id = obj_root.at("instance_id").As<double>().value();
47+
// Precedence: direct 'need_defaults' field overrides inference from
48+
// 'defaults'.
3649
if (obj_root.find("need_defaults") != obj_root.end()) {
3750
params.need_defaults = obj_root.at("need_defaults").As<bool>().value();
3851
} else {
39-
// Infer need_defaults from 'defaults' field
52+
// If 'need_defaults' is not present, infer from 'defaults' field.
4053
if (obj_root.find("defaults") != obj_root.end()) {
4154
auto defaults_val = obj_root.at("defaults").As<std::string>().value();
4255
if (defaults_val.get() == "required") {
4356
params.need_defaults = true;
57+
} else if (defaults_val.get() == "optional" ||
58+
defaults_val.get() == "without") {
59+
params.need_defaults = false;
4460
}
4561
}
4662
}
@@ -59,8 +75,8 @@ KvsParameters map_to_params(const std::string &data) {
5975
"_default.json";
6076
std::ifstream defaults_file(defaults_path);
6177
if (!defaults_file.good()) {
62-
throw std::runtime_error{"map_to_params Defaults file missing: " +
63-
defaults_path};
78+
throw ScenarioError(score::mw::per::kvs::ErrorCode::KvsFileReadError,
79+
"Defaults file missing: " + defaults_path);
6480
}
6581
}
6682
}

‎tests/cpp_test_scenarios/src/main.cpp‎

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,13 @@
1+
#include "internal/error.hpp"
2+
#include "score/result/result.h"
3+
4+
// Custom exception type for error code propagation
5+
class ScenarioError : public std::runtime_error {
6+
public:
7+
score::mw::per::kvs::ErrorCode code;
8+
ScenarioError(score::mw::per::kvs::ErrorCode code, const std::string &msg)
9+
: std::runtime_error(msg), code(code) {}
10+
};
111
#include <iostream>
212
#include <memory>
313
#include <string>
@@ -92,21 +102,24 @@ int main(int argc, char **argv) {
92102
// print_scenarios(root_group);
93103

94104
run_cli_app(raw_arguments, test_context);
95-
} catch (const std::runtime_error &ex) {
96-
std::string msg = ex.what();
97-
if (msg.find("Defaults file missing") != std::string::npos ||
98-
msg.find("Failed to parse JSON data") != std::string::npos ||
99-
msg.find("JsonParserError") != std::string::npos ||
100-
msg.find("malformed") != std::string::npos ||
101-
msg.find("parse error") != std::string::npos ||
102-
msg.find("invalid") != std::string::npos) {
103-
std::cerr << "[EXCEPTION] Critical error: " << msg << std::endl;
105+
} catch (const ScenarioError &ex) {
106+
// Robust error code-based exit
107+
using score::mw::per::kvs::ErrorCode;
108+
switch (ex.code) {
109+
case ErrorCode::KvsFileReadError:
110+
case ErrorCode::KvsHashFileReadError:
111+
case ErrorCode::JsonParserError:
112+
case ErrorCode::ValidationFailed:
113+
std::cerr << "[EXCEPTION] Critical error: " << ex.what() << std::endl;
104114
return 101;
105-
} else {
106-
std::cerr << "[EXCEPTION] Non-critical runtime error: " << msg
115+
default:
116+
std::cerr << "[EXCEPTION] Non-critical runtime error: " << ex.what()
107117
<< std::endl;
108118
return -1;
109119
}
120+
} catch (const std::runtime_error &ex) {
121+
std::cerr << "[EXCEPTION] std::runtime_error: " << ex.what() << std::endl;
122+
return -1;
110123
} catch (const std::exception &ex) {
111124
std::cerr << "[EXCEPTION] std::exception: " << ex.what() << std::endl;
112125
return -1;

0 commit comments

Comments
 (0)