mw::com: missing test cases#707
Conversation
KrishaDeshkool
force-pushed
the
improve_test_case_fpr2
branch
from
fdced1d to
56954c2
Compare
Add unit tests for to_string(MethodType), covering kMethod/kGet/kSet/kUnknown and the out-of-range "Invalid" fallback.
KrishaDeshkool
force-pushed
the
improve_test_case_fpr2
branch
3 times, most recently
from
82b1c31 to
c00b95f
Compare
KrishaDeshkool
force-pushed
the
improve_test_case_fpr2
branch
from
c00b95f to
6be28aa
Compare
KrishaDeshkool
requested review from
LittleHuba,
bemerybmw,
castler,
crimson11,
hoe-jo and
limdor
as code owners
|
|
||
| TEST(MethodTypeToStringTest, ReturnsMethodForKMethod) | ||
| { | ||
| // Given a MethodType of kMethod |
There was a problem hiding this comment.
Nitpick which doesn't have to be fixed but just for future reference, the given part is about setting up the environment / context for the test. In this case, the MethodType isn't part of the setup but is what's actually being tested. So it would be more correct to say: "When converting a MethodType of kMethod to string" without a given section.
| // When calling Get() | ||
| auto result = field.Get(); | ||
|
|
||
| // Then the call is delegated to the underlying method binding and succeeds |
There was a problem hiding this comment.
The expecting parts should go with the expectations above:
// Expecting that the binding returns a buffer for a return value and then calls the method
EXPECT_CALL(get_method_binding_mock_, GetReturnValueBuffer(0U)); EXPECT_CALL(get_method_binding_mock_, DoCall(0U));
// When calling Get()
auto result = field.Get();
// Then the call succeeds
There was a problem hiding this comment.
Same for all the other tests
| EXPECT_CALL(get_method_binding_mock_, DoCall(0U)); | ||
|
|
||
| // When calling Get() | ||
| auto result = field.Get(); |
There was a problem hiding this comment.
Can you also put a value in the return buffer and check that it's returned in result?
| for (const auto method_type : {MethodType::kGet, MethodType::kSet}) | ||
| { | ||
| auto proxy_method = | ||
| ProxyMethodBindingFactory<MethodSignature>::Create(handle, *this->proxy_, kDummyFieldName, method_type); |
There was a problem hiding this comment.
I guess we should also have a death test in case MethodType::kUnknown is provided (if we don't already have one)
2 participants
No description provided.