-
Notifications
You must be signed in to change notification settings - Fork 594
Change integration test harness to support scaling tests #1500
Change integration test harness to support scaling tests #1500
Conversation
@@ -86,6 +86,9 @@ public void execute(Tuple tuple) { | |||
collector.emit(Constants.INTEGRATION_TEST_CONTROL_STREAM_ID, | |||
tuple, | |||
new Values(Constants.INTEGRATION_TEST_TERMINAL)); | |||
} else { | |||
LOG.info(String.format( | |||
"Received a terminals, waiting to receive %s more", terminalsToReceive)); |
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.
it should be Received a terminal or Received terminals
@@ -14,18 +14,18 @@ | |||
# The location of default configure file | |||
DEFAULT_TEST_CONF_FILE = "integration-test/src/python/test_runner/resources/test.json" | |||
|
|||
RETRY_ATTEMPTS = 15 | |||
RETRY_ATTEMPTS = 25 |
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.
why the change from 15 to 25?
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.
When there are legit failures retrying 25 times is excessive and it takes over 4 minutes of results checking per test. Even 15 times seems high. If we have to retry anything this many times for simple, small test topologies, something is wrong.
That said, I can keep it if our tests are known to be too flaky when set below 25.
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.
Most of the times the integration tests not flaky, I believe.
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.
That's what I thought. Let's reduce it and see if it causes problems.
All comments addressed and CI is passing. Let me know if anyone has additional comments. |
LGTM 👍 |
* Change int test harness to support scaling tests * fix arg parsing for python tests * change retry back to 25 * fix bug during iteration * fix topo args bug * clean up log messages * reduce attempts back to 15
* Change int test harness to support scaling tests * fix arg parsing for python tests * change retry back to 25 * fix bug during iteration * fix topo args bug * clean up log messages * reduce attempts back to 15
Made a number of changes to support scaling tests:
updateArgs
, the test is considered a scaling (i.e., update) testMultiPhaseTestSpout
which outputs tuples, polls fortopology_updated
state changes on thehttp_server
and outputs more. This class relies onHttpGetCondition
that polls for expected state.topology_started
state.Apologies that this PR got large, but it was a hard one for me to break up.
There is still more work to be done to handle tests the scale down. Currently the aggregator bolt waits for teminal tuples from all upstream instances so removing one leaves it hanging. Will tackle that in a separate review.