OAK-11166 -- Remove asserts for flaky tests in VersionGarbageCollecto… #1759
Conversation
…rIT and BranchCommitGCTest
|
I have approved the run in the pipeline to see the result of tests. |
|
I probably missed this : what was the fix done that allows this? |
|
@stefan-egli adding code to drop tables for RDB H2 solved some issues previous generated. |
reschke
left a comment
reschke
left a comment
There was a problem hiding this comment.
as @stefan-egli said - this only make sense if the underlying problems were fixed.
|
I added code in BranchCommitGCTest to fix some flaky tests that had assumeTrue in it. |
| if (gcType == FullGCMode.ORPHANS_EMPTYPROPS_BETWEEN_CHECKPOINTS_NO_UNMERGED_BC | ||
| || gcType == FullGCMode.ORPHANS_EMPTYPROPS_BETWEEN_CHECKPOINTS_WITH_UNMERGED_BC) { | ||
| // temporarily skip due to flakyness | ||
| continue; | ||
| } | ||
| if (f.getName().equals("Memory") || f.getName().startsWith("RDB")) { | ||
| // then only run NONE and EMPTY_PROPS, cause we are rolling EMPTY_PROPS first | ||
| if (gcType != FullGCMode.NONE && gcType != FullGCMode.EMPTYPROPS | ||
| && gcType != FullGCMode.GAP_ORPHANS_EMPTYPROPS) { | ||
| // temporarily skip due to slowness | ||
| continue; | ||
| } | ||
| } |
There was a problem hiding this comment.
is this necessary? I'm aware we have the same over in VersionGarbageCollectorIT - but that was introduced based on some test flakyness. But for BranchCommitGCTest adding this would now disable fully functioning tests.
There was a problem hiding this comment.
This is not necessary. I will revert the changes.
| } | ||
| } | ||
|
|
||
| public static void assertEventually(Runnable r, long timeoutMillis) { |
There was a problem hiding this comment.
+1 for an assertEventually
|
|
||
| @Test | ||
| public void testBundledPropUnmergedBCGC() throws Exception { | ||
| assumeTrue(fullGcMode != FullGCMode.ORPHANS_EMPTYPROPS_KEEP_ONE_ALL_PROPS); |
There was a problem hiding this comment.
this was introduced due to this flaky test failure which happened on mongo - and IIUC then there's no fix for this. So the flakyness is likely still to be there. What's the idea of still enabling it?
There was a problem hiding this comment.
I will revert the changes. I don't have a fix
| // OAK-10869: | ||
| assumeTrue(fullGcMode != FullGCMode.ORPHANS_EMPTYPROPS_KEEP_ONE_ALL_PROPS); |
There was a problem hiding this comment.
This was introduced as a result of the mentioned flaky failure on mongo. Thus same question: why re-add it without any underlying test fix?
There was a problem hiding this comment.
I can close this PR and create a new one after further investigation
There was a problem hiding this comment.
+1, might make sense to get the assertEventually improvement in and split this one here to a new PR
There was a problem hiding this comment.
I reverted deleted assumeTrue statements and keep assertEventually changes.
|
I will close this PR. Adding assertEventually will not solve FullGC modes that are flaky(disabled with assertTrue statement). |
4 participants
Remove asserts for flaky tests in VersionGarbageCollectorIT and BranchCommitGCTest