Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set of fixes to unit-tests code #23071

Merged
merged 7 commits into from
Oct 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions src/app/tests/TestReadInteraction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1565,8 +1565,6 @@ void TestReadInteraction::TestSubscribeRoundtrip(nlTestSuite * apSuite, void * a
delegate.mGotEventResponse = false;
delegate.mNumAttributeResponse = 0;

printf("HereHere\n");

err = engine->GetReportingEngine().SetDirty(dirtyPath1);
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
err = engine->GetReportingEngine().SetDirty(dirtyPath2);
Expand Down Expand Up @@ -1840,13 +1838,16 @@ void TestReadInteraction::TestSubscribeWildcard(nlTestSuite * apSuite, void * ap
err = engine->GetReportingEngine().SetDirty(dirtyPath);
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);

ctx.DrainAndServiceIO();

//
// Not sure why I had to add this, and didn't have cycles to figure out why.
ATmobica marked this conversation as resolved.
Show resolved Hide resolved
// Tracked in Issue #17528.
// We need to DrainAndServiceIO() until attribute callback will be called.
// This is not correct behavior and is tracked in Issue #17528.
//
ctx.DrainAndServiceIO();
int last;
do
{
last = delegate.mNumAttributeResponse;
ctx.DrainAndServiceIO();
ATmobica marked this conversation as resolved.
Show resolved Hide resolved
} while (last != delegate.mNumAttributeResponse);

NL_TEST_ASSERT(apSuite, delegate.mGotReport);
// Mock endpoint3 has 13 attributes in total, and we subscribed twice.
Expand Down
4 changes: 2 additions & 2 deletions src/controller/tests/TestEventChunking.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -542,10 +542,10 @@ nlTestSuite sSuite =

} // namespace

int TestReadChunkingTests()
int TestEventChunkingTests()
{
gSuite = &sSuite;
return chip::ExecuteTestsWithContext<TestContext>(&sSuite);
}

CHIP_REGISTER_TEST_SUITE(TestReadChunkingTests)
CHIP_REGISTER_TEST_SUITE(TestEventChunkingTests)
4 changes: 2 additions & 2 deletions src/controller/tests/data_model/TestRead.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2642,8 +2642,8 @@ void TestReadInteraction::TestReadHandler_MultipleSubscriptionsWithDataVersionFi
numSuccessCalls == (app::InteractionModelEngine::kReadHandlerPoolSize + 1);
});

ChipLogError(Zcl, "Success call cnt: %u (expect %u) subscription cnt: %u (expect %u)", numSuccessCalls,
uint32_t(app::InteractionModelEngine::kReadHandlerPoolSize + 1), numSubscriptionEstablishedCalls,
ChipLogError(Zcl, "Success call cnt: %" PRIu32 " (expect %" PRIu32 ") subscription cnt: %" PRIu32 " (expect %" PRIu32 ")",
numSuccessCalls, uint32_t(app::InteractionModelEngine::kReadHandlerPoolSize + 1), numSubscriptionEstablishedCalls,
uint32_t(app::InteractionModelEngine::kReadHandlerPoolSize + 1));

NL_TEST_ASSERT(apSuite, numSuccessCalls == (app::InteractionModelEngine::kReadHandlerPoolSize + 1));
Expand Down
1 change: 1 addition & 0 deletions src/inet/tests/TestInetCommonPosix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
#include "TestInetCommon.h"
#include "TestInetCommonOptions.h"

#include <assert.h>
#include <errno.h>
#include <vector>

Expand Down
65 changes: 64 additions & 1 deletion src/lib/support/CHIPArgParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,11 @@ bool ParseArgs(const char * progName, int argc, char * const argv[], OptionSet *
OptionSet * curOptSet;
OptionDef * curOpt;
bool handlerRes;
#if CHIP_CONFIG_NON_POSIX_LONG_OPT
int lastOptIndex = 0;
int subOptIndex = 0;
int currentOptIndex = 0;
#endif // CHIP_CONFIG_NON_POSIX_LONG_OPT

// The getopt() functions do not support recursion, so exit immediately with an
// error if called recursively.
Expand Down Expand Up @@ -345,7 +350,36 @@ bool ParseArgs(const char * progName, int argc, char * const argv[], OptionSet *
// Attempt to match the current option argument (argv[optind]) against the defined long and short options.
optarg = nullptr;
optopt = 0;
id = getopt_long(argc, argv, shortOpts, longOpts, &optIndex);
#if CHIP_CONFIG_NON_POSIX_LONG_OPT
// to check if index has changed
lastOptIndex = currentOptIndex;
// optind will not increment on error, this is why we need to keep track of the current option
// this is for use when getopt_long fails to find the option and we need to print the error
currentOptIndex = optind;
// if it's the first run, optind is not set and we need to find the first option ourselves
if (!currentOptIndex)
{
while (currentOptIndex < argc)
{
currentOptIndex++;
if (*argv[currentOptIndex] == '-')
{
break;
}
}
}
// similarly we need to keep track of short opts index for groups like "-fba"
// if the index has not changed that means we are still analysing the same group
if (lastOptIndex != currentOptIndex)
{
subOptIndex = 0;
}
else
{
subOptIndex++;
}
#endif // CHIP_CONFIG_NON_POSIX_LONG_OPT
id = getopt_long(argc, argv, shortOpts, longOpts, &optIndex);

// Stop if there are no more options.
if (id == -1)
Expand All @@ -356,10 +390,35 @@ bool ParseArgs(const char * progName, int argc, char * const argv[], OptionSet *
{
if (ignoreUnknown)
continue;
#if CHIP_CONFIG_NON_POSIX_LONG_OPT
// getopt_long doesn't tell us if the option which failed to match is long or short so check
bool isLongOption = false;
if (strlen(argv[currentOptIndex]) > 2 && argv[currentOptIndex][1] == '-')
{
isLongOption = true;
}
if (optopt == 0 || isLongOption)
{
// getopt_long function incorrectly treats unknown long option as short opt group
if (subOptIndex == 0)
{
PrintArgError("%s: Unknown option: %s\n", progName, argv[currentOptIndex]);
}
}
else if (optopt == '?')
{
PrintArgError("%s: Unknown option: -%c\n", progName, argv[currentOptIndex][subOptIndex + 1]);
}
else
{
PrintArgError("%s: Unknown option: -%c\n", progName, optopt);
}
#else
if (optopt != 0)
PrintArgError("%s: Unknown option: -%c\n", progName, optopt);
else
PrintArgError("%s: Unknown option: %s\n", progName, argv[optind - 1]);
#endif // CHIP_CONFIG_NON_POSIX_LONG_OPT
goto done;
}

Expand All @@ -369,7 +428,11 @@ bool ParseArgs(const char * progName, int argc, char * const argv[], OptionSet *
{
// NOTE: with the way getopt_long() works, it is impossible to tell whether the option that
// was missing an argument was a long option or a short option.
#if CHIP_CONFIG_NON_POSIX_LONG_OPT
PrintArgError("%s: Missing argument for %s option\n", progName, argv[currentOptIndex]);
#else
PrintArgError("%s: Missing argument for %s option\n", progName, argv[optind - 1]);
#endif // CHIP_CONFIG_NON_POSIX_LONG_OPT
goto done;
}

Expand Down
2 changes: 1 addition & 1 deletion src/lib/support/jsontlv/TlvJson.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ void InsertKeyValue(Json::Value & json, const KeyContext & keyContext, T val)
}
else if (keyContext.keyType == KeyContext::kStructField)
{
snprintf(keyBuf, sizeof(keyBuf), "%" PRIu32, keyContext.key);
snprintf(keyBuf, sizeof(keyBuf), "%u", keyContext.key);
json[keyBuf] = val;
}
else
Expand Down
6 changes: 4 additions & 2 deletions src/lib/support/tests/TestBytesToHex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -433,8 +433,10 @@ const nlTest sTests[] = {
NL_TEST_DEF("TestBytesToHexErrors", TestBytesToHexErrors), //
NL_TEST_DEF("TestBytesToHexUint64", TestBytesToHexUint64), //
NL_TEST_DEF("TestHexToBytesAndUint", TestHexToBytesAndUint), //
NL_TEST_DEF("TestLogBufferAsHex", TestLogBufferAsHex), //
NL_TEST_SENTINEL() //
#ifdef CHIP_PROGRESS_LOGGING
NL_TEST_DEF("TestLogBufferAsHex", TestLogBufferAsHex), //
#endif
NL_TEST_SENTINEL() //
};

} // namespace
Expand Down
9 changes: 7 additions & 2 deletions src/lib/support/tests/TestCHIPArgParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ static size_t sCallbackRecordCount = 0;
static OptionDef sOptionSetA_Defs[] =
{
{ "foo", kNoArgument, '1' },
{ "bar", kNoArgument, 1001 },
{ "bar", kNoArgument, 1002 },
{ "baz", kArgumentRequired, 'Z' },
{ }
};
Expand Down Expand Up @@ -350,7 +350,7 @@ static void SimpleParseTest_VariousShortAndLongWithArgs()
VerifyHandleOptionCallback(0, __FUNCTION__, &sOptionSetA, '1', "--foo", nullptr);
VerifyHandleOptionCallback(1, __FUNCTION__, &sOptionSetB, 1000, "--run", "run-value");
VerifyHandleOptionCallback(2, __FUNCTION__, &sOptionSetB, 's', "-s", nullptr);
VerifyHandleOptionCallback(3, __FUNCTION__, &sOptionSetA, 1001, "--bar", nullptr);
VerifyHandleOptionCallback(3, __FUNCTION__, &sOptionSetA, 1002, "--bar", nullptr);
VerifyHandleOptionCallback(4, __FUNCTION__, &sOptionSetA, '1', "-1", nullptr);
VerifyHandleOptionCallback(5, __FUNCTION__, &sOptionSetA, 'Z', "-Z", "baz-value");
VerifyHandleOptionCallback(6, __FUNCTION__, &sOptionSetB, 1000, "--run", "run-value-2");
Expand Down Expand Up @@ -779,7 +779,12 @@ int TestCHIPArgParser(void)
UnknownOptionTest_UnknownShortOptionBeforeKnown();
UnknownOptionTest_UnknownLongOptionAfterArgs();
UnknownOptionTest_IgnoreUnknownShortOption();

/* Skip this test because the parser successfully captures all the options
but the error reporting is incorrect in this case due to long_opt limitations */
#ifndef CHIP_CONFIG_NON_POSIX_LONG_OPT
UnknownOptionTest_IgnoreUnknownLongOption();
#endif // !CHIP_CONFIG_NON_POSIX_LONG_OPT

MissingValueTest_MissingShortOptionValue();
MissingValueTest_MissingLongOptionValue();
Expand Down
5 changes: 4 additions & 1 deletion src/transport/raw/tests/TestPeerAddress.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,11 @@ void TestToString(nlTestSuite * inSuite, void * inContext)
{
IPAddress::FromString("1223::3456:789a", ip);
PeerAddress::UDP(ip, 8080).ToString(buff);
// IPV6 does not specify case
int res1 = strcmp(buff, "UDP:[1223::3456:789a]:8080");
int res2 = strcmp(buff, "UDP:[1223::3456:789A]:8080");

NL_TEST_ASSERT(inSuite, !strcmp(buff, "UDP:[1223::3456:789a]:8080"));
NL_TEST_ASSERT(inSuite, (!res1 || !res2));
}

{
Expand Down