-
Notifications
You must be signed in to change notification settings - Fork 309
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
HPCC-33499 Update esdltests failure identification and reporting #19576
base: candidate-9.10.x
Are you sure you want to change the base?
HPCC-33499 Update esdltests failure identification and reporting #19576
Conversation
- Use CPPUNIT macros for testing success and printing messages on failure. - Print timing and other non-failure test information using DBGLOG to leverage the -v verbose option. Re-direct script context tracing away from stdout for the same reason. Some test cases generate expected errors. Since the underlying libxml2 prints parsing and XPath errors and warnings to stderr, you will see messages that look like failures even when all tests succeed. A separate ticket is opened to suppress these errors and warnings: HPCC-33544. Signed-off-by: Terrence Asselin <terrence.asselin+copilot@lexisnexisrisk.com>
Jira Issue: https://hpccsystems.atlassian.net//browse/HPCC-33499 Jirabot Action Result: |
- Add assert to catch tests that should throw an exception but don't. - Update the implicit prefix test to remove an incorrect assumption that using the implicitly-defined "n" prefix is required when a default namespace is used. Signed-off-by: Terrence Asselin <terrence.asselin+copilot@lexisnexisrisk.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check with @AttilaVamos to see if the added use of DBGLOG
in passing tests is acceptable, or if test output should be limited to failure messages.
testing/unittests/esdltests.cpp
Outdated
} | ||
fprintf(stdout, "\nTxSummary-%s-%s) '%s' close enough to '%s'\n", prefix, test, actualText.str(), expectedText); | ||
DBGLOG("\nTxSummary-%s-%s) '%s' close enough to '%s'\n", prefix, test, actualText.str(), expectedText); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As the author of this test, I'm not sure this line adds value. It can probably be removed. If retained, consider removing unnecessary EOLN characters. DBGLOG should already give us that. Double check this for all output messages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok removed that one. Confirming with Attila about the couple others.
testing/unittests/esdltests.cpp
Outdated
VStringBuffer msg(" %2u: %d/%d/%s\n", ++idx, he.msgAudience, he.msgClass, he.msg.str()); | ||
accumulatedMessage.append(msg); | ||
} | ||
CPPUNIT_ASSERT_EQUAL_MESSAGE(accumulatedMessage.str(), expected.size(), sink->history.size()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe CPPUNIT_FAIL? The check on 2222 guarantees the test failure. A casual reader might not realize this is guaranteed to fail and expect the test to continue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to FAIL
testing/unittests/esdltests.cpp
Outdated
printf("\nOOOPs Exception %d - %s\n", E->errorCode(), E->errorMessage(msg).str()); | ||
throw E; | ||
StringBuffer m; | ||
VStringBuffer exceptionMessage("Unexpected Exception: code=%d, message=%s", E->errorCode(), E->errorMessage(m).str()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trivial, but with the exception of the exception caught in runTest
all exceptions are unexpected. Even there, the only messaging will be for unexpected exceptions. Not sure that Unexpected
is necessary every time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
Remove some unnecessary output and clean up messages. Signed-off-by: Terrence Asselin <terrence.asselin+copilot@lexisnexisrisk.com>
Confirmed with Attila that the OBT and smoketest don't use the |
Some test cases generate expected errors. Since the underlying libxml2 prints parsing and XPath errors and warnings to stderr, you will see messages that look like failures even when all tests succeed. A separate ticket is opened to suppress these errors and warnings: HPCC-33544.
Type of change:
Checklist:
Smoketest:
Testing: