Shbikram/de conflict tests#2207
Conversation
| * Example: "rawEcdhKeyringItem" becomes "rawEcdhKeyringItem-482913" | ||
| */ | ||
| public static String uniquePk(String base) { | ||
| return base + "-" + TEST_PREFIX; |
There was a problem hiding this comment.
nit: variable says prefix but this is suffix
| assertNotNull(ase.getErrorCode()); | ||
| assertNotNull(ase.getErrorType()); | ||
| assertNotNull(ase.getMessage()); | ||
| } catch (java.util.concurrent.RejectedExecutionException e) { |
There was a problem hiding this comment.
I prefer to let it fail for visibility of the failure
There was a problem hiding this comment.
What are we gaining from this visibility apart from having to retry? Is there any action item on failure?
There was a problem hiding this comment.
Is RejectedExecutionException expected failure from this test?
There was a problem hiding this comment.
Yes. In parallel run two threads are scheduled to make call to DDB. If first one fails before other thread is even submitted, the executor shuts down and second submit fails leading to RejectedExecutionException.
Since this is a failure scenario, the first thread will always fail. In that case, it depends on whether the second thread is scheduled or not which is out of our control.
There was a problem hiding this comment.
praise: just having random PK will largely reduce flaky test.
There was a problem hiding this comment.
I like that partition key is random which will reduce flaky CI. This will end up creating huge DDB. So, we should put clean up code to remove the item created by examples. I would send the pk name from test then add clean up code in test to keep actual examples clean.
There was a problem hiding this comment.
There are multiple solutions to clean up.
But for now, I think we punt that issue, and accept these changes.
Addressing CI clean up is an issue best left for another day.
There was a problem hiding this comment.
Let me attempt a afterClass in each file. If that takes too long, we can park that and come back with another cleanup story.
|
You might want to change PR title |
| */ | ||
| public class ExampleUtils { | ||
|
|
||
| private static final String TEST_PREFIX = String.valueOf( |
There was a problem hiding this comment.
| private static final String TEST_PREFIX = String.valueOf( | |
| private static final String TEST_SUFFIX = String.valueOf( |
| * Example: "rawEcdhKeyringItem" becomes "rawEcdhKeyringItem-482913" | ||
| */ | ||
| public static String uniquePk(String base) { | ||
| return base + "-" + TEST_PREFIX; |
There was a problem hiding this comment.
| return base + "-" + TEST_PREFIX; | |
| return base + "-" + TEST_SUFFIX; |
| import java.util.concurrent.ThreadLocalRandom; | ||
|
|
||
| /** | ||
| * Shared utilities for examples. The TEST_PREFIX provides a unique suffix |
There was a problem hiding this comment.
| * Shared utilities for examples. The TEST_PREFIX provides a unique suffix | |
| * Shared utilities for examples. The TEST_SUFFIX provides a unique suffix |
There was a problem hiding this comment.
There are multiple solutions to clean up.
But for now, I think we punt that issue, and accept these changes.
Addressing CI clean up is an issue best left for another day.
* chore(java): deconflicts the test run across various Java and OS versions * chore(java): attempts to deconflict tests across different java and OS versions * chore(java): fixing bugs in test scenario leading to failures * chore(java): fixes format issue and also addressing ddb parallel test run failure * chore(java): Adds afterTest cleanup method to clean DDB items created in test run
* chore(java): deconflicts the test run across various Java and OS versions * chore(java): attempts to deconflict tests across different java and OS versions * chore(java): fixing bugs in test scenario leading to failures * chore(java): fixes format issue and also addressing ddb parallel test run failure * chore(java): Adds afterTest cleanup method to clean DDB items created in test run
3 participants
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.