Skip to content

Commit fec6d40

Browse files
committed
mw/com: Update NonTrivialConstructors test
Fail the test using FailTest instead of propagating the errors up through function return types which makes signatures and calling more complex and can result in tests not failing when they should if the error code is not handled correctly by the parent layer.
1 parent 732dfcc commit fec6d40

9 files changed

Lines changed: 81 additions & 121 deletions

File tree

score/mw/com/test/methods/methods_test_resources/method_consumer.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ class MethodConsumer
6161
WITH_COPY
6262
};
6363

64-
bool CreateProxy(InstanceSpecifier instance_specifier);
64+
void CreateProxy(InstanceSpecifier instance_specifier, const std::string& failure_message_prefix);
6565

6666
bool CallMethodWithInArgsAndReturn(const std::int32_t input_argument_a,
6767
const std::int32_t input_argument_b,
@@ -80,9 +80,9 @@ class MethodConsumer
8080
};
8181

8282
template <typename Proxy>
83-
bool MethodConsumer<Proxy>::CreateProxy(InstanceSpecifier instance_specifier)
83+
void MethodConsumer<Proxy>::CreateProxy(InstanceSpecifier instance_specifier, const std::string& failure_message_prefix)
8484
{
85-
return proxy_container_.CreateProxy(std::move(instance_specifier));
85+
proxy_container_.CreateProxy(std::move(instance_specifier), failure_message_prefix);
8686
}
8787

8888
template <typename Proxy>

score/mw/com/test/methods/non_trivial_constructors/consumer.cpp

Lines changed: 42 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
********************************************************************************/
1313
#include "score/mw/com/test/methods/non_trivial_constructors/consumer.h"
1414

15+
#include "score/mw/com/test/common_test_resources/fail_test.h"
1516
#include "score/mw/com/test/methods/methods_test_resources/process_synchronizer.h"
1617
#include "score/mw/com/test/methods/methods_test_resources/proxy_container.h"
1718
#include "score/mw/com/test/methods/non_trivial_constructors/test_method_datatype.h"
@@ -33,14 +34,16 @@ const std::string kInterprocessNotificationShmPath{"/non_trivial_constructors_te
3334
const InstanceSpecifier kInstanceSpecifier =
3435
InstanceSpecifier::Create(std::string{"test/methods/non_trivial_constructors/MethodSignature"}).value();
3536

36-
bool CallMethodWithInArgsAndReturn(NonTrivialConstructorProxy& proxy)
37+
void CallMethodWithInArgsAndReturn(NonTrivialConstructorProxy& proxy, const std::string& failure_message_prefix)
3738
{
38-
auto call_result = [&proxy]() -> score::Result<impl::MethodReturnTypePtr<NonTriviallyConstructibleType>> {
39+
auto call_result =
40+
[&proxy, &failure_message_prefix]() -> score::Result<impl::MethodReturnTypePtr<NonTriviallyConstructibleType>> {
3941
std::cout << "\n=== Test: with_in_args_and_return (zero-copy) ===" << std::endl;
4042
auto allocated_args_result = proxy.with_in_args_and_return.Allocate();
4143
if (!allocated_args_result.has_value())
4244
{
43-
std::cerr << "Consumer: Could not allocate method args: " << allocated_args_result.error() << std::endl;
45+
FailTest(
46+
failure_message_prefix, " Consumer: Could not allocate method args: ", allocated_args_result.error());
4447
return Unexpected(allocated_args_result.error());
4548
}
4649

@@ -49,31 +52,31 @@ bool CallMethodWithInArgsAndReturn(NonTrivialConstructorProxy& proxy)
4952
}();
5053
if (!call_result.has_value())
5154
{
52-
std::cerr << "Consumer: with_in_args_and_return call failed: " << call_result.error() << std::endl;
53-
return false;
55+
FailTest(failure_message_prefix, " Consumer: with_in_args_and_return call failed: ", call_result.error());
56+
return;
5457
}
5558
const auto return_value = *(call_result.value());
5659

5760
// Since provider adds the two input args which are both initialized with kInitialValue
5861
const auto expected_return_value = NonTriviallyConstructibleType{} + NonTriviallyConstructibleType{};
5962
if (return_value != expected_return_value)
6063
{
61-
std::cerr << "Consumer: Expected " << expected_return_value << " but got " << return_value << std::endl;
62-
return false;
64+
FailTest(failure_message_prefix, " Consumer: Expected ", expected_return_value, " but got ", return_value);
65+
return;
6366
}
6467

6568
std::cout << "Consumer: with_in_args_and_return returned correct result: " << return_value << std::endl;
66-
return true;
6769
}
6870

69-
bool CallMethodWithInArgsOnly(NonTrivialConstructorProxy& proxy)
71+
void CallMethodWithInArgsOnly(NonTrivialConstructorProxy& proxy, const std::string& failure_message_prefix)
7072
{
71-
auto call_result = [&proxy]() -> Result<void> {
73+
auto call_result = [&proxy, &failure_message_prefix]() -> Result<void> {
7274
std::cout << "\n=== Test: with_in_args_only (zero-copy) ===" << std::endl;
7375
auto allocated_args_result = proxy.with_in_args_only.Allocate();
7476
if (!allocated_args_result.has_value())
7577
{
76-
std::cerr << "Consumer: Could not allocate method args: " << allocated_args_result.error() << std::endl;
78+
FailTest(
79+
failure_message_prefix, " Consumer: Could not allocate method args: ", allocated_args_result.error());
7780
return Unexpected(allocated_args_result.error());
7881
}
7982

@@ -83,90 +86,76 @@ bool CallMethodWithInArgsOnly(NonTrivialConstructorProxy& proxy)
8386
}();
8487
if (!call_result.has_value())
8588
{
86-
std::cerr << "Consumer: with_in_args_only call failed: " << call_result.error() << std::endl;
87-
return false;
89+
FailTest(failure_message_prefix, " Consumer: with_in_args_only call failed: ", call_result.error());
90+
return;
8891
}
8992

9093
std::cout << "Consumer: with_in_args_only returned without error" << std::endl;
91-
return true;
9294
}
9395

94-
bool CallMethodWithReturnOnly(NonTrivialConstructorProxy& proxy)
96+
void CallMethodWithReturnOnly(NonTrivialConstructorProxy& proxy, const std::string& failure_message_prefix)
9597
{
9698
std::cout << "\n=== Test: with_return_only (copy call) ===" << std::endl;
9799
const auto call_result = proxy.with_return_only();
98100
if (!call_result.has_value())
99101
{
100-
std::cerr << "Consumer: with_return_only call failed: " << call_result.error() << std::endl;
101-
return false;
102+
FailTest(failure_message_prefix, " Consumer: with_return_only call failed: ", call_result.error());
103+
return;
102104
}
103105
const auto return_value = *(call_result.value());
104106

105107
const auto expected_return_value = NonTriviallyConstructibleType{};
106108
if (return_value != expected_return_value)
107109
{
108-
std::cerr << "Consumer: Expected " << expected_return_value << " but got " << return_value << std::endl;
109-
return false;
110+
FailTest(failure_message_prefix, " Consumer: Expected ", expected_return_value, " but got ", return_value);
111+
return;
110112
}
111113

112114
std::cout << "Consumer: with_return_only returned correct result: " << return_value << std::endl;
113-
return true;
114115
}
115116

116117
} // namespace
117118

118-
int run_consumer()
119+
void run_consumer()
119120
{
120121
auto process_synchronizer_result = ProcessSynchronizer::Create(kInterprocessNotificationShmPath);
121122
if (!(process_synchronizer_result.has_value()))
122123
{
123-
std::cerr << "Methods non_trivial_constructors consumer failed: Could not create ProcessSynchronizer"
124-
<< std::endl;
125-
return EXIT_FAILURE;
124+
FailTest("Methods non_trivial_constructors consumer failed: Could not create ProcessSynchronizer");
125+
return;
126126
}
127127

128+
// Set an exit function to notify the provider in case of failure in calls to FailTest, so that it does not wait
129+
// indefinitely for the consumer to finish.
130+
ExitFunction process_synchronizer_guard{[&process_synchronizer_result]() {
131+
process_synchronizer_result->Notify();
132+
}};
133+
SetFailTestExitFunction(process_synchronizer_guard);
134+
135+
// Create a scope exit guard to clear the fail test exit function, so that it cannot be called after the
136+
// process_synchronizer has been destroyed.
137+
utils::ScopeExit fail_test_exit_function_guard{[&process_synchronizer_result]() {
138+
ClearTestExitFunction();
139+
process_synchronizer_result->Notify();
140+
}};
141+
128142
ProxyContainer<NonTrivialConstructorProxy> consumer{};
129143

130144
// Step 1. Find service and create proxy
131145
std::cout << "\nConsumer: Step 1" << std::endl;
132-
if (!consumer.CreateProxy(kInstanceSpecifier))
133-
{
134-
std::cerr << "Methods non_trivial_constructors consumer failed: CreateProxy" << std::endl;
135-
process_synchronizer_result->Notify();
136-
return EXIT_FAILURE;
137-
}
146+
consumer.CreateProxy(kInstanceSpecifier, "non_trivial_constructors");
138147

139148
// Step 2. Call zero-copy method with InArgs and Return
140149
std::cout << "\nConsumer: Step 2" << std::endl;
141-
if (!CallMethodWithInArgsAndReturn(consumer.GetProxy()))
142-
{
143-
std::cerr << "Methods non_trivial_constructors consumer failed: CallMethodWithInArgsAndReturnZeroCopy"
144-
<< std::endl;
145-
process_synchronizer_result->Notify();
146-
return EXIT_FAILURE;
147-
}
150+
CallMethodWithInArgsAndReturn(consumer.GetProxy(), "non_trivial_constructors");
148151

149152
// Step 3. Call zero-copy method with InArgs only
150153
std::cout << "\nConsumer: Step 3" << std::endl;
151-
if (!CallMethodWithInArgsOnly(consumer.GetProxy()))
152-
{
153-
std::cerr << "Methods non_trivial_constructors consumer failed: CallMethodWithInArgsOnlyZeroCopy" << std::endl;
154-
process_synchronizer_result->Notify();
155-
return EXIT_FAILURE;
156-
}
154+
CallMethodWithInArgsOnly(consumer.GetProxy(), "non_trivial_constructors");
157155

158156
// Step 4. Call method with return only with copy
159157
std::cout << "\nConsumer: Step 4" << std::endl;
160-
if (!CallMethodWithReturnOnly(consumer.GetProxy()))
161-
{
162-
std::cerr << "Methods non_trivial_constructors consumer failed: CallMethodWithReturnOnly" << std::endl;
163-
process_synchronizer_result->Notify();
164-
return EXIT_FAILURE;
165-
}
166-
167-
process_synchronizer_result->Notify();
168-
169-
return EXIT_SUCCESS;
158+
CallMethodWithReturnOnly(consumer.GetProxy(), "non_trivial_constructors");
170159
}
171160

172161
} // namespace score::mw::com::test

score/mw/com/test/methods/non_trivial_constructors/consumer.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
namespace score::mw::com::test
1717
{
1818

19-
int run_consumer();
19+
void run_consumer();
2020

2121
} // namespace score::mw::com::test
2222

score/mw/com/test/methods/non_trivial_constructors/integration_test/BUILD

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,3 @@ integration_test(
3030
filesystem = ":filesystem",
3131
flaky = True, # TODO: Remove flaky tag once test is stable: Refer to issue 386
3232
)
33-
34-
test_suite(
35-
name = "component_tests",
36-
tests = [
37-
":non_trivial_constructors_test",
38-
],
39-
)

score/mw/com/test/methods/non_trivial_constructors/main_consumer.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,5 +18,6 @@ int main(int argc, const char** argv)
1818
{
1919
score::mw::com::test::SetupAssertHandler();
2020
score::mw::com::runtime::InitializeRuntime(argc, argv);
21-
return score::mw::com::test::run_consumer();
21+
score::mw::com::test::run_consumer();
22+
return EXIT_SUCCESS;
2223
}

score/mw/com/test/methods/non_trivial_constructors/main_provider.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,5 +29,6 @@ int main(int argc, const char** argv)
2929
std::cerr << "Unable to set signal handler for SIGINT and/or SIGTERM, cautiously continuing\n";
3030
}
3131

32-
return score::mw::com::test::run_provider(stop_source.get_token());
32+
score::mw::com::test::run_provider(stop_source.get_token());
33+
return EXIT_SUCCESS;
3334
}

0 commit comments

Comments
 (0)