load-test: delete testing data is only needed at end of tests, since each test uses unique resource prefix #686
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ach test uses unique resource prefix Signed-off-by: Wesley Pettit <[email protected]>
PettitWesley
changed the title
PettitWesley
commented
Jun 13, 2023
| def delete_testing_data(session): | ||
| # All testing data related to the plugin option will be deleted | ||
| if OUTPUT_PLUGIN == 'cloudwatch': |
There was a problem hiding this comment.
the "delete_testing_resources" only runs once, so its no longer necessary to check the plugin here. The delete happens just once at the end of the cloudwatch tests in the main "delete_testing_resources" routine.
PettitWesley
commented
Jun 13, 2023
| @@ -0,0 +1,2 @@ | |||
| # no content needed for stdout test | |||
There was a problem hiding this comment.
this change is just meant for maintainence to explain why this file is empty
zwj102030
approved these changes
Jun 13, 2023
DrewZhang13
approved these changes
Jun 13, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why is this change needed?
Last week I made a change to speed up the tests and fix a timeout that had occurred by starting all tests first, then checking all results. Basically, run all in parallel vs serial.
However, the code currently deletes testing data in between each test runs, which no longer works since they run in parallel now.
When do we need to delete testing data?
There's already a
delete_testing_resourcesphase after all testing runs are completed. That's the right place to delete test data.The previous code to delete between test runs was entirely unnecessary. Each test run uses a unique log stream in CW or unique S3 key in S3.
Why is this change safe?
The previous code to delete between test runs was entirely unnecessary. Each test run uses a unique log stream in CW or unique S3 key in S3.
How was this change tested?
I used the commit here to check that the python syntax is correct and that the generated task defs are correct: PettitWesley@d5fd7c1
Since we do not have a test pipeline already available, its not possible to run the load tests prior to PR merge.
This change is needed to unblock the 2.31.12 release which multiple customers are waiting on.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.