Skip to content

Commit e8a05a3

Browse files
marcosbentomarcosbento
committed
Correct handling of --help/--version with ECF_SSL
Re ECFLOW-2081
1 parent d0683a8 commit e8a05a3

4 files changed

Lines changed: 171 additions & 27 deletions

File tree

‎libs/client/src/ecflow/client/ClientOptions.cpp‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,19 @@ Cmd_ptr ClientOptions::parse(const CommandLine& cl, ClientEnvironment* env) cons
137137
env->set_debug(true);
138138
}
139139

140+
// If the `--help` option is provided, display the requested help message and exit
141+
if (vm.count("help")) {
142+
std::string topic = vm["help"].as<std::string>();
143+
std::cout << Help{*desc_, topic};
144+
return nullptr;
145+
}
146+
147+
// If the `--version` option is provided, display the version information and exit
148+
if (vm.count("version")) {
149+
std::cout << Version::description() << "\n";
150+
return nullptr;
151+
}
152+
140153
// Check to see if host or port, specified. This will override the environment variables
141154
std::string host, port;
142155
if (vm.count("port")) {
@@ -248,16 +261,6 @@ Cmd_ptr ClientOptions::parse(const CommandLine& cl, ClientEnvironment* env) cons
248261
// For example:
249262
// --server_load // this is sent to server
250263
// --server_load=<path> // no command returned, command executed by client
251-
if (vm.count("help")) {
252-
std::string topic = vm["help"].as<std::string>();
253-
std::cout << Help{*desc_, topic};
254-
return client_request;
255-
}
256-
257-
if (vm.count("version")) {
258-
std::cout << Version::description() << "\n";
259-
exit(0);
260-
}
261264

262265
std::ostringstream ss;
263266
ss << print_variable_map(vm) << "\n";

‎libs/client/src/ecflow/client/ClientOptions.hpp‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,17 @@ class ClientOptions {
4040

4141
~ClientOptions();
4242

43-
/// parse the arguments and create the client request that is to be sent
44-
/// to the server. Will throw std::runtime_error if invalid arguments specified
45-
Cmd_ptr parse(const CommandLine& cl, ClientEnvironment*) const;
43+
///
44+
/// @brief Parse the command line arguments and create the client request that is to be sent to the server.
45+
///
46+
/// @param cl The command line arguments to parse
47+
/// @param environment The client environment, which may be used by some commands to construct the client request
48+
/// @return A pointer to the client request to be sent to the server.
49+
/// The pointer can be nullptr if the command is client specific (e.g. --help, --version)
50+
/// and does not need to send a request to the server.
51+
/// @throw std::runtime_error if invalid arguments are specified
52+
///
53+
Cmd_ptr parse(const CommandLine& cl, ClientEnvironment* environment) const;
4654

4755
private:
4856
CtsCmdRegistry cmdRegistry_;

‎libs/test/foolproof/TestFoolproof.Basic.cpp‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,94 @@ BOOST_AUTO_TEST_SUITE(S_Foolproof)
1717

1818
BOOST_AUTO_TEST_SUITE(T_Basic)
1919

20+
BOOST_AUTO_TEST_CASE(test_client_help) {
21+
ECF_NAME_THIS_TEST();
22+
23+
using namespace foolproof::scaffold;
24+
25+
using command_t = RunClient::CommandHelp;
26+
27+
auto version = std::regex(R"---(Ecflow( \([^)]+\))? version\([^)]+\))---");
28+
auto commands = std::regex(R"---(Commands:)---");
29+
30+
{
31+
auto client = RunClient{}.execute(command_t{});
32+
BOOST_CHECK(client.ok());
33+
auto& c = client.value();
34+
35+
ECF_TEST_DBG("Output of --help:\n" << c.stdout_buffer);
36+
37+
BOOST_CHECK(c.stdout_contains(version));
38+
BOOST_CHECK(c.stdout_contains(commands));
39+
}
40+
{
41+
auto ecf_ssl = MakeEnvironmentVariable{}.with("ECF_SSL", "1").create();
42+
43+
auto client = RunClient{}.execute(command_t{});
44+
BOOST_CHECK(client.ok());
45+
auto& c = client.value();
46+
47+
ECF_TEST_DBG("Output of --help:\n" << c.stdout_buffer);
48+
49+
BOOST_CHECK(c.stdout_contains(version));
50+
BOOST_CHECK(c.stdout_contains(commands));
51+
}
52+
{
53+
auto ecf_ssl = MakeEnvironmentVariable{}.with("ECF_SSL", "x").create();
54+
55+
auto client = RunClient{}.execute(command_t{});
56+
BOOST_CHECK(client.ok());
57+
auto& c = client.value();
58+
59+
ECF_TEST_DBG("Output of --help:\n" << c.stdout_buffer);
60+
61+
BOOST_CHECK(c.stdout_contains(version));
62+
BOOST_CHECK(c.stdout_contains(commands));
63+
}
64+
}
65+
66+
BOOST_AUTO_TEST_CASE(test_client_version) {
67+
ECF_NAME_THIS_TEST();
68+
69+
using namespace foolproof::scaffold;
70+
71+
using command_t = RunClient::CommandVersion;
72+
73+
auto version = std::regex(R"---(Ecflow( \([^)]+\))? version\([^)]+\))---");
74+
75+
{
76+
auto client = RunClient{}.execute(command_t{});
77+
BOOST_CHECK(client.ok());
78+
auto& c = client.value();
79+
80+
ECF_TEST_DBG("Output of --version:\n" << c.stdout_buffer);
81+
82+
BOOST_CHECK(c.stdout_contains(version));
83+
}
84+
{
85+
auto ecf_ssl = MakeEnvironmentVariable{}.with("ECF_SSL", "1").create();
86+
87+
auto client = RunClient{}.execute(command_t{});
88+
BOOST_CHECK(client.ok());
89+
auto& c = client.value();
90+
91+
ECF_TEST_DBG("Output of --version:\n" << c.stdout_buffer);
92+
93+
BOOST_CHECK(c.stdout_contains(version));
94+
}
95+
{
96+
auto ecf_ssl = MakeEnvironmentVariable{}.with("ECF_SSL", "x").create();
97+
98+
auto client = RunClient{}.execute(command_t{});
99+
BOOST_CHECK(client.ok());
100+
auto& c = client.value();
101+
102+
ECF_TEST_DBG("Output of --version:\n" << c.stdout_buffer);
103+
104+
BOOST_CHECK(c.stdout_contains(version));
105+
}
106+
}
107+
20108
BOOST_AUTO_TEST_CASE(test_e2e_ping) {
21109
ECF_NAME_THIS_TEST();
22110

‎libs/test/foolproof/scaffold/Provisioning.hpp‎

Lines changed: 59 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -761,32 +761,57 @@ struct Client
761761
bool stdout_contains(const std::string& expected) const {
762762
return stdout_buffer.find(expected) != std::string::npos;
763763
}
764+
bool stdout_contains(const std::regex& expected) const {
765+
std::smatch match;
766+
std::regex_search(stdout_buffer, match, expected);
767+
return !match.empty();
768+
}
764769
};
765770

766771
class RunClient {
767772
public:
768773
using host_t = Host;
769774
using port_t = Port;
770775

776+
struct CommandHelp
777+
{
778+
static constexpr bool contacts_server = false;
779+
780+
std::vector<std::string> options() const { return {"--help"}; }
781+
};
782+
783+
struct CommandVersion
784+
{
785+
static constexpr bool contacts_server = false;
786+
787+
std::vector<std::string> options() const { return {"--version"}; }
788+
};
789+
771790
struct CommandPing
772791
{
773-
std::vector<std::string> options() const { return {"--ping", "-d"}; }
792+
static constexpr bool contacts_server = true;
774793

775-
void process_output(const std::string& output) const {}
794+
std::vector<std::string> options() const { return {"--ping", "-d"}; }
776795
};
777796

778797
struct CommandGet
779798
{
799+
static constexpr bool contacts_server = true;
800+
780801
std::vector<std::string> options() const { return {"--get"}; }
781802
};
782803

783804
struct CommandGetState
784805
{
806+
static constexpr bool contacts_server = true;
807+
785808
std::vector<std::string> options() const { return {"--get_state"}; }
786809
};
787810

788811
struct CommandLoad
789812
{
813+
static constexpr bool contacts_server = true;
814+
790815
explicit CommandLoad(fs::path defs)
791816
: defs_{defs} {}
792817

@@ -797,6 +822,8 @@ class RunClient {
797822

798823
struct CommandReplace
799824
{
825+
static constexpr bool contacts_server = true;
826+
800827
explicit CommandReplace(fs::path defs, std::string node)
801828
: defs_{defs},
802829
node_{std::move(node)} {}
@@ -809,6 +836,8 @@ class RunClient {
809836

810837
struct CommandDelete
811838
{
839+
static constexpr bool contacts_server = true;
840+
812841
explicit CommandDelete(std::string path)
813842
: path_{path} {}
814843

@@ -819,6 +848,8 @@ class RunClient {
819848

820849
struct CommandUpdateLabel
821850
{
851+
static constexpr bool contacts_server = true;
852+
822853
explicit CommandUpdateLabel(std::string path, std::string label, std::string value)
823854
: path_{path},
824855
label_{label},
@@ -833,6 +864,8 @@ class RunClient {
833864

834865
struct CommandReloadWhitelist
835866
{
867+
static constexpr bool contacts_server = true;
868+
836869
explicit CommandReloadWhitelist() {}
837870

838871
std::vector<std::string> options() const { return {"--reloadwsfile"}; }
@@ -885,16 +918,26 @@ class RunClient {
885918
const User* user,
886919
const Directory* cwd,
887920
const Command& command) {
888-
BOOST_REQUIRE_MESSAGE(host != nullptr, "The server host is non-null");
889-
BOOST_REQUIRE_MESSAGE(port != nullptr, "The server port is non-null");
890-
BOOST_REQUIRE_MESSAGE(cwd != nullptr, "The working directory is non-null");
921+
if constexpr (Command::contacts_server) {
922+
BOOST_REQUIRE_MESSAGE(host != nullptr, "The server host is non-null");
923+
BOOST_REQUIRE_MESSAGE(port != nullptr, "The server port is non-null");
924+
BOOST_REQUIRE_MESSAGE(cwd != nullptr, "The working directory is non-null");
925+
}
891926

892927
auto client_path = find_ecflow_client_path();
893928

894929
BOOST_REQUIRE_MESSAGE(!client_path.empty(), "The ecflow client path is non-empty");
895930
BOOST_REQUIRE_MESSAGE(fs::exists(client_path), "The ecflow client executable exist at " << client_path);
896931

897-
auto options = std::vector<std::string>{"--host", host->value(), "--port", std::to_string(port->value())};
932+
auto options = std::vector<std::string>{};
933+
if (host != nullptr) {
934+
options.push_back("--host");
935+
options.push_back(host->value());
936+
}
937+
if (port != nullptr) {
938+
options.push_back("--port");
939+
options.push_back(std::to_string(port->value()));
940+
}
898941
if (user != nullptr) {
899942
options.push_back("--user");
900943
options.push_back(user->username);
@@ -905,7 +948,9 @@ class RunClient {
905948
options.push_back(option);
906949
}
907950

908-
auto ecflow_client = Process(client_path, options, cwd->path());
951+
auto wd = cwd != nullptr ? cwd->path() : fs::current_path();
952+
953+
auto ecflow_client = Process(client_path, options, wd);
909954

910955
auto print = [](const auto& executable, const auto& options) {
911956
std::string buffer = "[ " + pretty_print_path(executable) + (options.empty() ? "" : ", ");
@@ -922,22 +967,22 @@ class RunClient {
922967
ECF_TEST_DBG("Executed " << print(client_path, options));
923968
ECF_TEST_DBG(" result: [OK]");
924969
ECF_TEST_DBG(" pid: " << ecflow_client.pid());
925-
ECF_TEST_DBG(" cwd: " << cwd->path().string());
926-
auto [stdout_buffer, stderr_buffer] = dump_client_execution_report(*cwd, ecflow_client);
970+
ECF_TEST_DBG(" cwd: " << wd.string());
971+
auto [stdout_buffer, stderr_buffer] = dump_client_execution_report(wd, ecflow_client);
927972
return Outcome<Client>::success(Client{r, stdout_buffer, stderr_buffer});
928973
}
929974
else {
930975
ECF_TEST_DBG("Executed " << print(client_path, options));
931976
ECF_TEST_DBG(" result: [FAIL]");
932977
ECF_TEST_DBG(" pid: " << ecflow_client.pid());
933-
ECF_TEST_DBG(" cwd: " << cwd->path().string());
934-
auto [stdout_buffer, stderr_buffer] = dump_client_execution_report(*cwd, ecflow_client);
978+
ECF_TEST_DBG(" cwd: " << wd.string());
979+
auto [stdout_buffer, stderr_buffer] = dump_client_execution_report(wd, ecflow_client);
935980
return Outcome<Client>::failure("ecflow_client failed, return code: " + std::to_string(r) +
936981
", stdout: " + stdout_buffer + ", stderr: " + stderr_buffer);
937982
}
938983
}
939984

940-
static std::tuple<std::string, std::string> dump_client_execution_report(const Directory& cwd,
985+
static std::tuple<std::string, std::string> dump_client_execution_report(const fs::path& wd,
941986
const Process& server) {
942987
auto out = server.read_stdout();
943988
auto err = server.read_stderr();
@@ -946,8 +991,8 @@ class RunClient {
946991
std::string{"ecflow_client__execution_report."} + std::to_string(server.pid()) + ".stdout.txt";
947992
std::string report_stderr_file =
948993
std::string{"ecflow_client__execution_report."} + std::to_string(server.pid()) + ".stderr.txt";
949-
fs::path report_stdout_path = cwd.path() / report_stdout_file;
950-
fs::path report_stderr_path = cwd.path() / report_stderr_file;
994+
fs::path report_stdout_path = wd / report_stdout_file;
995+
fs::path report_stderr_path = wd / report_stderr_file;
951996

952997
{
953998
std::ofstream ofs(report_stdout_path.c_str());

0 commit comments

Comments
 (0)