stefan-egli commented on code in PR #1759:
URL: https://github.com/apache/jackrabbit-oak/pull/1759#discussion_r1795057353
##########
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java:
##########
@@ -2290,9 +2286,6 @@ public void testBundlingAndLatestSplit() throws Exception
{
@Test
public void testBundledPropUnmergedBCGC() throws Exception {
- assumeTrue(fullGcMode !=
FullGCMode.ORPHANS_EMPTYPROPS_KEEP_ONE_ALL_PROPS);
Review Comment:
this was introduced due to [this flaky test
failure](https://issues.apache.org/jira/browse/OAK-10843?focusedCommentId=17850687&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17850687)
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?
##########
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/BranchCommitGCTest.java:
##########
@@ -790,4 +812,34 @@ private void assertNoEmptyProperties() {
}
}
+ public static void assertEventually(Runnable r, long timeoutMillis) {
Review Comment:
+1 for an `assertEventually`
##########
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/BranchCommitGCTest.java:
##########
@@ -82,6 +85,19 @@ public static java.util.Collection<Object[]> params() throws
IOException {
for (Object[] fixture : AbstractDocumentStoreTest.fixtures()) {
DocumentStoreFixture f = (DocumentStoreFixture)fixture[0];
for (FullGCMode gcType : FullGCMode.values()) {
+ 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;
+ }
+ }
Review 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.
##########
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java:
##########
@@ -2659,8 +2652,6 @@ public void testGCDeletedPropsWithDryRunMode() throws
Exception {
@Test
public void testDeletedPropsAndUnmergedBCWithCollisionWithDryRunMode()
throws Exception {
- // OAK-10869:
- assumeTrue(fullGcMode !=
FullGCMode.ORPHANS_EMPTYPROPS_KEEP_ONE_ALL_PROPS);
Review 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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
