Hey Damian,
Thank you for your feedback and for the latest patches.
Also your patch in the mail: "[Openmama-dev] [PATCH] UNIT TESTS:
Addition of variable NOT_IMPLEMENTED status checking."
from what what I've seen, is extremely useful. It gives ability to
have flexible tests for each bridge implementation and for each
phase in the development cycle.
Regards,
Guy
toggle quoted messageShow quoted text
On 3/12/2013 1:46 PM, Damian Maguire wrote:
Hey Guy,
I've added your changes to the
feature-unit-test branch, with some minor formatting changes
and excluding the disabling of GetSendSubjectValid test. Commit ID: 45c945ed1baefb0b6cc1787391da6dff25198b25
Cheers,
Damian
Damian Maguire – Senior R&D
and OpenMAMA Specialist
IntercontinentalExchange | NYSE Technologies
24-26 Adelaide Exchange | Belfast, BT2 8GD
Tel: +44 2890 822 282 (ext: 452161) | Mob: +44 7540 204
077
Hi,
We've now run a complete set of the payload unit
tests on the RMDS. This mail notes some issues
in the general payload tests,
payloadgeneraltests.cpp, and suggest some
improved tests for which a patch is offered.
There is no direct support for Windows VS
solution/project for the payload tests. We
tested the the unit-test in our modified
environment. We can't test directly on a cloned
OpenMAMA repository without modification, but we
supply all the information needed to apply the
changes.
Fixed tests
PayloadGeneralTests.IterDestroyValid
where:
result =
aBridge->msgPayloadIterDestroy(testIter);
//testIter == NULL
EXPECT_EQ (result, MAMA_STATUS_OK);
why:
Test does not check valid iterator. An
additional test is proposed in the patch.
PayloadGeneralTests.IterDestroyInValidIter
where:
result =
aBridge->msgPayloadIterDestroy(NULL);
EXPECT_EQ (result, MAMA_STATUS_OK);
why:
testIter is NULL, so the expected
result is MAMA_STATUS_NULL_ARG. The qpid
implementation returns that too.
qpidmsgPayloadIter_destroy (msgPayloadIter iter)
{
qpidIterImpl* impl = (qpidIterImpl*)iter;
if (NULL == iter)
{
return MAMA_STATUS_NULL_ARG;
}
solution:
should change to
EXPECT_EQ (result, MAMA_STATUS_NULL_ARG);
PayloadGeneralTests.IterNextInValidIter
where:
output =
aBridge->msgPayloadIterNext(NULL, testField,
testPayload);
if (output == NULL)
FAIL();
why:
The test incorrectly considers NULL output
as failure but passing a null iterator would
expect a null output
see qpid implementation:
msgFieldPayload
qpidmsgPayloadIter_next
(msgPayloadIter iter,
msgFieldPayload field,
msgPayload msg)
{
qpidIterImpl* impl =
(qpidIterImpl*) iter;
qpidmsgPayloadImpl* msgImpl =
(qpidmsgPayloadImpl*) msg;
/* Field will be NULL on first call so
don't check for it */
if (NULL == iter || NULL == msg)
{
return NULL;
}
solution:
change to:
if (output != NULL)
PayloadGeneralTests.IterBeginInValidIter
where:
output =
aBridge->msgPayloadIterBegin(NULL, testField,
testPayload);
if (output == NULL)
FAIL();
why:
The test incorrectly considers NULL output
as failure but passing a null iterator would
expect a null output
see also qpid implementation:
qpidmsgPayloadIter_begin
(msgPayloadIter iter,
msgFieldPayload field,
msgPayload msg)
{
qpidIterImpl* impl =
(qpidIterImpl*)iter;
qpidmsgPayloadImpl* msgImpl =
(qpidmsgPayloadImpl*)msg;
if (NULL == iter || NULL == msg)
{
return NULL;
}
solution:
change to
if (output != NULL)
PayloadGeneralTests.IterHasNextInValidPayload
where:
output =
aBridge->msgPayloadIterHasNext(testIter,
NULL);
EXPECT_EQ (output, MAMA_STATUS_NULL_ARG);
why:
msgPayloadIterHasNext() returns boolean the
test is wrong and expects a status result
MAMA_STATUS_NULL_ARG
solution:
change:
EXPECT_EQ (output, 0);
PayloadGeneralTests.IterAssociateValid
where:
result =
aBridge->msgPayloadIterAssociate(&testIter,
testPayload); //#1
EXPECT_EQ (result, MAMA_STATUS_OK);
result =
aBridge->msgPayloadIterCreate(&testIter,
testPayload);
EXPECT_EQ (result, MAMA_STATUS_OK);
result =
aBridge->msgPayloadIterAssociate(&testIter,
testPayload); //#2
EXPECT_EQ (result, MAMA_STATUS_OK);
why:
#1
1. Because testIter is passed to the
function as an address (see &). It should be
passed by value.
2. Also testIter is associated before
being created
#2
1. because testIter is passed to the
function as an address (see &). It should be
passed by value.
solution:
The Create() call is now the first call.
Ampersands are stripped from testIter in the
call to msgPayloadIterAssociate(...)
I provide an additional test
"IterAssociateTwiceValid" which checks if
association works for 2 payloads reusing one
iterator.
PayloadGeneralTests.GetSendSubjectInValidMsg
why:
The test considers the function as
not-implemented, but actually should test if
message passed as NULL.
where
TEST_F(PayloadGeneralTests,
GetSendSubjectInValidMsg)
{
const char** mySubject
(NULL);
result =
aBridge->msgPayloadGetSendSubject(NULL,
mySubject);
EXPECT_EQ (result,
MAMA_STATUS_NOT_IMPLEMENTED);
}
solution:
change to:
EXPECT_EQ (result, MAMA_STATUS_NULL_ARG);
PayloadGeneralTests.IterHasNextValid
where:
output =
aBridge->msgPayloadIterHasNext(&testIter,
testPayload);
EXPECT_EQ (output, true);
why:
testIter parameter is passed as an address
(with &). Shouldn't pass by reference.
solution:
change to
output =
aBridge->msgPayloadIterHasNext(testIter,
testPayload);
PayloadGeneralTests.IterHasNextInValidPayload
where:
output =
aBridge->msgPayloadIterHasNext(&testIter,
NULL);
EXPECT_EQ (output, MAMA_STATUS_NULL_ARG);
why:
testIter parameter is passed as an address
(with &). Shouldn't pass by reference.
solution:
change to
output =
aBridge->msgPayloadIterHasNext(testIter,
NULL);
PayloadGeneralTests.CreateFromByteBufferInValidBufferLength
where:
result =
aBridge->msgPayloadGetFieldAsString(testPayload,
&testName, testFid, &testBuf, NULL);
EXPECT_EQ (result, MAMA_STATUS_OK);
why:
1. argument testBuf is passed as an address
(see &) It should be passed by value. even
though testBuf is only 'char' (in the test) it
is confusing. It should be 'char *'
2. the last argument should be zero. NULL is
just improper even though valid. expected status
value should be 'invalid argument'.
solution:
1.
change declaration to:
char testBuf[256];
//originally was just char!
2.
change the call to:
result =
aBridge->msgPayloadGetFieldAsString(testPayload,
&testName, testFid, testBuf, 0);
3.
and then expected value:
EXPECT_EQ (result, MAMA_STATUS_INVALID_ARG);
PayloadGeneralTests.GetFieldAsStringValid
where:
result =
aBridge->msgPayloadGetFieldAsString(testPayload,
&testName, testFid, testBuf, testLen);
EXPECT_EQ (result, MAMA_STATUS_OK);
why:
1. argument testBuf is passed as an address
(see &) should be passed by value. even
though testBuf is only 'char' it can is
confusing. It should be 'char *'
solution:
1. change declaraion to:
char testBuf[256];
2. change to:
result =
aBridge->msgPayloadGetFieldAsString(testPayload,
&testName, testFid, testBuf, testLen);
PayloadGeneralTests.GetFieldAsStringInValidName
where:
result =
aBridge->msgPayloadGetFieldAsString(testPayload,
NULL, testFid, testBuf, testLen);
EXPECT_EQ (result, MAMA_STATUS_OK);
why:
1. argument testBuf is passed as an address
(see &) should be passed by value. even
though testBuf is only 'char' it can confuse
anyone! it should be 'char *'
solution:
1. change declaraion to:
char testBuf[256];
2. change to:
result =
aBridge->msgPayloadGetFieldAsString(testPayload,
NULL, testFid, testBuf, testLen);
PayloadGeneralTests.GetSendSubjectValid
This is not a useful test (expects unimplemented
function). We suggest it should be disabled for
the moment.
Proposed new tests:
PayloadGeneralTests.ToStringValidConsistent
what it does:
tests that check if the string that comes
out of 2 identical payloads is consistent.
PayloadGeneralTests.IterNextValidConsistent
what it does:
tests that traverse FIXED amount of times
over the payload and expect to get ALL the
payloads fields in the end.
PayloadGeneralTests.IterAssociateTwiceValid
what it does:
test that check association (reuse) of 1
iterator with 2 payloads.
Regards,
Guy - Tick42
This
message may contain confidential information and is intended for
specific recipients unless explicitly noted otherwise. If you
have reason to believe you are not an intended recipient of this
message, please delete it and notify the sender. This message
may not represent the opinion of IntercontinentalExchange Group,
Inc. (ICE), NYSE Euronext or any of their subsidiaries or
affiliates, and does not constitute a contract or guarantee.
Unencrypted electronic mail is not secure and the recipient of
this message is expected to provide safeguards from viruses and
pursue alternate means of communication where privacy or a
binding message is desired.
|