From a8daf4514c0fbdf5886d28b6d7d4f5e7c2145e71 Mon Sep 17 00:00:00 2001 From: Jacqueline Deans Date: Thu, 15 Dec 2022 12:53:39 -0600 Subject: [PATCH] Improvements to SIM_test_varserv (#1414) * Add freeze test to SIM_test_varserv; make mode test more comprehensive --- .../models/test_client/test_client.cpp | 302 +++++++++++++----- 1 file changed, 223 insertions(+), 79 deletions(-) diff --git a/test/SIM_test_varserv/models/test_client/test_client.cpp b/test/SIM_test_varserv/models/test_client/test_client.cpp index f859a3fa..982806bf 100644 --- a/test/SIM_test_varserv/models/test_client/test_client.cpp +++ b/test/SIM_test_varserv/models/test_client/test_client.cpp @@ -15,6 +15,11 @@ #define SOCKET_BUF_SIZE 20480 +#define DOUBLE_TOL 1e-5 + +// Pretend that gtest was kind enough to provide an EXPECT_FEQ operator with a tolerance +#define EXPECT_FEQ(a,b) EXPECT_LE(fabs(a - b), DOUBLE_TOL) + class Socket { public: @@ -337,26 +342,16 @@ TEST_F (VariableServerTest, Cycle) { } double cycle = 1.0; - double tolerance = 0.01; + double tolerance = 1e-5; int num_cycles = 5; // Challenge: no loops allowed // I've been reading about lamdbas and when you have a hammer........ // Test: compare the differences in the returned sim time, make sure the difference - // between them are equal to what var_cycle has been set to within some tolerance - // Problem: setting a tolerance empirically is subject to fluctuactions in the environment, - // which could cause the test to fail even though everthing is functioning correctly - // For example, the original version of tolerances here passed in all the CI pipelines - // except Mac, which for some reason was returning much more variation in cycle time - - // Instead, calculate the average cycle time for a few cycles, and make sure this - // value is closer to the current set cycle time than the previous set cycle time. - // This gives us less information before, we are basically only testing that - // var_cycle is actually changing the cycle time, instead of testing that the - // cycle time is being closely adhered to, but it shouldn't fail the pipeline unnecessarily - // And testing something is better than nothing - + // between them are equal to var_cycle + // In copy mode VS_COPY_SCHEDULED and write mode VS_WRITE_WHEN_COPY, we should see exactly the correct copy rate + // Use a very small tolerance to compensate for floating point error auto parse_message_for_sim_time = [](const std::string& message) { // For this case the message will be @@ -370,51 +365,37 @@ TEST_F (VariableServerTest, Cycle) { }; // Tail recursion, just for fun - std::function& )> record_cycle_times = [&] (int n_cycles, double last_sim_time, std::vector& cycle_times) { + std::function compare_cycle = [&] (int n_cycles, double last_sim_time) { if (n_cycles <= 0) return; double sim_time = parse_message_for_sim_time(socket.receive()); - cycle_times.push_back(sim_time - last_sim_time); - record_cycle_times(n_cycles-1, sim_time, cycle_times); + EXPECT_FEQ((sim_time - last_sim_time), cycle) << "Expected cycle: " << cycle << " Measured cycle: " << (sim_time - last_sim_time); + compare_cycle(n_cycles-1, sim_time); }; - // Does this count as tail recursion if the last thing is technically a return instead of a call to sum_vec? - std::function&, size_t)> sum_vec = [&] (std::vector& vec, size_t index) -> double { - if (index == vec.size()) - return 0; - - return vec[index] + sum_vec(vec, index+1); - }; - - auto measure_cycle = [&] (double cycle_length, int iterations) -> double { - std::string command = "trick.var_cycle(" + std::to_string(cycle_length) + ")\n"; + auto run_cycle_test = [&] () { + std::string command = "trick.var_cycle(" + std::to_string(cycle) + ")\n"; socket << command; + // Give it a cycle to update + socket.receive(); double sim_time = parse_message_for_sim_time(socket.receive()); - std::vector cycle_times; - record_cycle_times(iterations, sim_time, cycle_times); - return sum_vec(cycle_times, 0) / cycle_times.size(); + compare_cycle(num_cycles, sim_time); }; - auto closer_to = [] (double expected, double other, double test_val) -> bool { - return (fabs(expected - test_val)) < (fabs(other - test_val)); - }; - - std::function&, size_t)> compare_cycle_times = [&] (std::vector& test_cycle_times, size_t index) { - if (index == test_cycle_times.size()) - return; - - double measured_cycle_time = measure_cycle(test_cycle_times[index], 5); - EXPECT_TRUE(closer_to(test_cycle_times[index], test_cycle_times[index-1], measured_cycle_time)) << "Expected time: " << test_cycle_times[index] << " Actual time: " << measured_cycle_time; - compare_cycle_times(test_cycle_times, index+1); - }; - - std::string command = "trick.var_add(\"time\")\n"; + std::string command = "trick.var_set_copy_mode(1)\ntrick.var_set_write_mode(1)\ntrick.var_add(\"time\")\n"; socket << command; - std::vector test_cycle_times = {0, 3.0, 0.1, 1.0, 0.5}; - compare_cycle_times(test_cycle_times, 1); + cycle = 0.1; + num_cycles = 5; + run_cycle_test(); + cycle = 0.5; + run_cycle_test(); + + cycle = 3.0; + num_cycles = 3; + run_cycle_test(); } @@ -447,6 +428,95 @@ TEST_F (VariableServerTest, Pause) { EXPECT_EQ(strcmp_IgnoringWhiteSpace(reply, expected), 0); } +TEST_F (VariableServerTest, Freeze) { + if (socket_status != 0) { + FAIL(); + } + + std::string reply; + std::string expected; + int mode; + long long frame_count; + long long freeze_frame_count; + + // Constants for clarity + const int MODE_RUN = 5; + const int MODE_FREEZE = 1; + + // lambda capture by refence is neat + auto parse_message_for_sim_mode_and_frames = [&](const std::string& message) { + // For this case the message will be + // 0\t\t\t\t + // Sets the local variables to new values + std::stringstream message_stream(message); + std::string token; + std::getline(message_stream, token, '\t'); + std::getline(message_stream, token, '\t'); + mode = std::stoi(token); + std::getline(message_stream, token, '\t'); + frame_count = std::stoll(token); + std::getline(message_stream, token, '\t'); + freeze_frame_count = std::stoll(token); + }; + + auto wait_for_mode_change = [&] (int expected_mode, int max_wait_iterations = 10) { + int iteration = 0; + while (iteration++ < max_wait_iterations && mode != expected_mode) { + parse_message_for_sim_mode_and_frames(socket.receive()); + } + }; + + // Send the sim mode, frame_count, freeze_frame_count, and sim_time + socket << "trick.var_add(\"trick_sys.sched.mode\")\ntrick.var_add(\"trick_sys.sched.frame_count\")\ntrick.var_add(\"trick_sys.sched.freeze_frame_count\")\n"; + parse_message_for_sim_mode_and_frames(socket.receive()); + + // Sim mode should be MODE_RUN == 5 + EXPECT_EQ(mode, MODE_RUN); + + // Set to Freeze mode + socket << "trick.exec_freeze()\n"; + + // The mode takes a little bit of time to change + wait_for_mode_change(MODE_FREEZE); + + // Sim mode should be MODE_FREEZE == 1 + ASSERT_EQ(mode, MODE_FREEZE); + + // Not sure what else to test in copy mode VS_COPY_ASYNC + + // Test VS_COPY_SCHEDULED - should see 1 message per frame + socket << "trick.var_set_copy_mode(1)\n"; + parse_message_for_sim_mode_and_frames(socket.receive()); + int num_tests = 5; + int prev_frame = 0; + for (int i = 0; i < num_tests; i++) { + prev_frame = freeze_frame_count; + parse_message_for_sim_mode_and_frames(socket.receive()); + EXPECT_EQ(freeze_frame_count - prev_frame, 1); + } + + + // Test VS_COPY_TOP_OF_FRAME, along with freeze frame multiplier and offset + socket << "trick.var_set_copy_mode(2)\ntrick.var_set_freeze_frame_multiple(3)\ntrick.var_set_freeze_frame_offset(1)\n"; + parse_message_for_sim_mode_and_frames(socket.receive()); + for (int i = 0; i < num_tests; i++) { + prev_frame = freeze_frame_count; + parse_message_for_sim_mode_and_frames(socket.receive()); + EXPECT_EQ(freeze_frame_count % 3, 1); + EXPECT_EQ(freeze_frame_count - prev_frame, 3); + } + + // Set the mode back to run or the next tests will get confused + socket << "trick.exec_run()\n"; + socket << "trick.var_set_copy_mode(0)\n"; + + // Wait for the mode to actually change + wait_for_mode_change(MODE_RUN); + + ASSERT_EQ(mode, MODE_RUN); +} + + TEST_F (VariableServerTest, CopyAndWriteModes) { if (socket_status != 0) { FAIL(); @@ -455,35 +525,78 @@ TEST_F (VariableServerTest, CopyAndWriteModes) { std::string reply; std::string expected; - // We're just checking that every combination of modes is functional - // We can't test that they perform their copying and writing in the correct place from here - // Default is 0 0 - socket << "trick.var_add(\"vsx.vst.a\")\ntrick.var_add(\"vsx.vst.b\")\n"; - socket >> reply; + int num_tests = 5; + double sim_time; + int frame_count; + std::string vars; + std::string command; + std::string test_vars_command = "trick.var_add(\"time\")\ntrick.var_add(\"trick_sys.sched.frame_count\")\n"; - expected = "0 97 98"; - EXPECT_EQ(strcmp_IgnoringWhiteSpace(reply, expected), 0); + double expected_cycle = 0.5; + command = "trick.var_cycle(" + std::to_string(expected_cycle) + ")\n"; + socket << command; + + auto parse_message = [&](const std::string& message) { + // For this case the message will be + // 0\t {s}\t\t\n + // Sets the local variables to new values + std::stringstream message_stream(message); + std::string token; + std::getline(message_stream, token, '\t'); + std::getline(message_stream, token, ' '); + sim_time = std::stod(token); + std::getline(message_stream, token, '\t'); + std::getline(message_stream, token, '\t'); + frame_count = std::stoll(token); + std::getline(message_stream, token, '\n'); + vars = token; + }; + + auto spin = [&](int wait_cycles = 5) { + socket.receive(); + }; + + // Check that every combination of modes is functional + // Check that reasonable times and frames are returned as well + // Default is VS_COPY_ASYNC=0 and VS_WRITE_ASYNC=0 + // This mode doesn't give us any guarantees about frame or cycle consistency, so we can't test it + // Just make sure it works + + command = test_vars_command + "trick.var_add(\"vsx.vst.a\")\ntrick.var_add(\"vsx.vst.b\")\n"; + socket << command; + parse_message(socket.receive()); + + expected = "97 98"; + EXPECT_EQ(strcmp_IgnoringWhiteSpace(vars, expected), 0) << "Received: " << vars << " Expected: " << expected; // Clear out anything else that's been sent - // I may need to write something else for this socket << "trick.var_pause()\n"; socket.clear_buffered_data(); // Copy mode 1 (VS_COPY_SCHEDULED) write mode 0 (VS_WRITE_ASYNC) - socket << "trick.var_clear()\ntrick.var_set_copy_mode(1)\ntrick.var_add(\"vsx.vst.c\")\ntrick.var_add(\"vsx.vst.d\")\ntrick.var_unpause()\n"; - socket >> reply; + command = "trick.var_clear()\n" + test_vars_command + "trick.var_set_copy_mode(1)\ntrick.var_add(\"vsx.vst.c\")\ntrick.var_add(\"vsx.vst.d\")\ntrick.var_unpause()\n"; + socket << command; // With copy mode VS_COPY_SCHEDULED and write mode VS_WRITE_ASYNC, the first reply will be all 0 since the main time to copy has not occurred yet. // Is this what we want? Maybe we should have more strict communication on whether the data has been staged so the first message isn't incorrect + spin(); // expected = "0 -1234 1234"; // EXPECT_EQ(strcmp_IgnoringWhiteSpace(reply, expected), 0); // std::cout << "\tExpected: " << expected << "\n\tActual: " << reply << std::endl; + expected = "-1234 1234"; + parse_message(socket.receive()); + EXPECT_EQ(strcmp_IgnoringWhiteSpace(vars, expected), 0) << "Received: " << vars << " Expected: " << expected; - socket >> reply; - - expected = "0 -1234 1234"; - EXPECT_EQ(strcmp_IgnoringWhiteSpace(reply, expected), 0); + // Test that we see a difference of exactly expected_cycle (with a tolerance for floating point issues) + int prev_frame = 0; + double prev_time = 0; + for (int i = 0; i < num_tests; i++) { + prev_time = sim_time; + parse_message(socket.receive()); + EXPECT_FEQ(sim_time - prev_time, expected_cycle); + EXPECT_EQ(strcmp_IgnoringWhiteSpace(vars, expected), 0) << "Received: " << vars << " Expected: " << expected; + } // Clear out anything else that's been sent socket << "trick.var_pause()\n"; @@ -491,15 +604,22 @@ TEST_F (VariableServerTest, CopyAndWriteModes) { // Copy mode 1 (VS_COPY_SCHEDULED) write mode 1 (VS_WRITE_WHEN_COPIED) - socket << "trick.var_clear()\ntrick.var_set_write_mode(1)\ntrick.var_add(\"vsx.vst.e\")\ntrick.var_add(\"vsx.vst.f\")\ntrick.var_unpause()\n"; + command = "trick.var_clear()\n" + test_vars_command + "trick.var_set_write_mode(1)\ntrick.var_add(\"vsx.vst.e\")\ntrick.var_add(\"vsx.vst.f\")\ntrick.var_unpause()\n"; + socket << command; - socket >> reply; - expected = "0 -123456 123456"; - EXPECT_EQ(strcmp_IgnoringWhiteSpace(reply, expected), 0); + parse_message(socket.receive()); + expected = "-123456 123456"; + EXPECT_EQ(strcmp_IgnoringWhiteSpace(vars, expected), 0) << "Received: " << vars << " Expected: " << expected; - socket >> reply; - expected = "0 -123456 123456"; - EXPECT_EQ(strcmp_IgnoringWhiteSpace(reply, expected), 0); + // Test that we still see a difference of exactly expected_cycle (with a tolerance for floating point issues) + prev_frame = 0; + prev_time = 0; + for (int i = 0; i < num_tests; i++) { + prev_time = sim_time; + parse_message(socket.receive()); + EXPECT_FEQ(sim_time - prev_time, expected_cycle); + EXPECT_EQ(strcmp_IgnoringWhiteSpace(vars, expected), 0) << "Received: " << vars << " Expected: " << expected; + } // Clear out anything else that's been sent socket << "trick.var_pause()\n"; @@ -507,17 +627,31 @@ TEST_F (VariableServerTest, CopyAndWriteModes) { // Copy mode 2 (VS_COPY_TOP_OF_FRAME) write mode 0 (VS_WRITE_ASYNC) - socket << "trick.var_clear()\ntrick.var_set_copy_mode(2)\ntrick.var_set_write_mode(0)\ntrick.var_add(\"vsx.vst.g\")\ntrick.var_add(\"vsx.vst.h\")\ntrick.var_unpause()\n"; + // Test frame_multiple and frame_offset as well, with stupid values + int frame_multiple = 13*5; + int frame_offset = 7; + command = "trick.var_clear()\n" + test_vars_command + "trick.var_set_copy_mode(2)\ntrick.var_set_write_mode(0)\ntrick.var_set_frame_multiple(" + std::to_string(frame_multiple) + ")\ntrick.var_set_frame_offset(" + std::to_string(frame_offset) + ")\ntrick.var_add(\"vsx.vst.g\")\ntrick.var_add(\"vsx.vst.h\")\ntrick.var_unpause()\n"; + socket << command; // Same issue as copy mode 1 write mode 0 - socket >> reply; + spin(); // expected = "0 -1234567 123456789"; // EXPECT_EQ(strcmp_IgnoringWhiteSpace(reply, expected), 0); // std::cout << "\tExpected: " << expected << "\n\tActual: " << reply << std::endl; - socket >> reply; - expected = "0 -1234567 123456789"; - EXPECT_EQ(strcmp_IgnoringWhiteSpace(reply, expected), 0); + parse_message(socket.receive()); + expected = "-1234567 123456789"; + EXPECT_EQ(strcmp_IgnoringWhiteSpace(vars, expected), 0) << "Received: " << vars << " Expected: " << expected; + + // Test that we see a difference in frame_count of exactly frame_multiple with an offset of frame_offset + prev_frame = 0; + for (int i = 0; i < num_tests; i++) { + prev_frame = frame_count; + parse_message(socket.receive()); + EXPECT_EQ(frame_count - prev_frame, frame_multiple); + EXPECT_EQ(frame_count % frame_multiple, frame_offset); + EXPECT_EQ(strcmp_IgnoringWhiteSpace(vars, expected), 0) << "Received: " << vars << " Expected: " << expected; + } // Clear out anything else that's been sent socket << "trick.var_pause()\n"; @@ -525,14 +659,24 @@ TEST_F (VariableServerTest, CopyAndWriteModes) { // Copy mode 2 (VS_COPY_TOP_OF_FRAME) write mode 1 (VS_WRITE_WHEN_COPIED) - socket << "trick.var_clear()\ntrick.var_set_copy_mode(2)\ntrick.var_set_write_mode(1)\ntrick.var_add(\"vsx.vst.i\")\ntrick.var_add(\"vsx.vst.j\")\ntrick.var_unpause()\n"; - socket >> reply; - expected = "0 1234.5677 -1234.56789"; - EXPECT_EQ(strcmp_IgnoringWhiteSpace(reply, expected), 0); + frame_multiple = 17*5; + frame_offset = 11; + command = "trick.var_clear()\n" + test_vars_command + "trick.var_set_copy_mode(2)\ntrick.var_set_write_mode(1)\ntrick.var_set_frame_multiple(" + std::to_string(frame_multiple) + ")\ntrick.var_set_frame_offset(" + std::to_string(frame_offset) + ")\ntrick.var_add(\"vsx.vst.i\")\ntrick.var_add(\"vsx.vst.j\")\ntrick.var_unpause()\n"; + socket << command; - socket >> reply; - expected = "0 1234.5677 -1234.56789"; - EXPECT_EQ(strcmp_IgnoringWhiteSpace(reply, expected), 0); + expected = "1234.5677 -1234.56789"; + parse_message(socket.receive()); + EXPECT_EQ(strcmp_IgnoringWhiteSpace(vars, expected), 0) << "Received: " << vars << " Expected: " << expected; + + // Test that we see a difference in frame_count of exactly frame_multiple with an offset of frame_offset + prev_frame = 0; + for (int i = 0; i < num_tests; i++) { + prev_frame = frame_count; + parse_message(socket.receive()); + EXPECT_EQ(frame_count - prev_frame, frame_multiple); + EXPECT_EQ(frame_count % frame_multiple, frame_offset); + EXPECT_EQ(strcmp_IgnoringWhiteSpace(vars, expected), 0) << "Received: " << vars << " Expected: " << expected; + } // Clear out anything else that's been sent socket << "trick.var_pause()\n";