OAK-11113 : performed cleanup after running CompressedDocumentPropert… #1749
Conversation
|
Alternate proposal: 009d68e |
| @@ -92,7 +97,7 @@ public void compressValueThrowsException() throws IOException { | |||
| CompressedDocumentPropertyState.setCompressionThreshold(DEFAULT_COMPRESSION_THRESHOLD); | |||
| CompressedDocumentPropertyState documentPropertyState = new CompressedDocumentPropertyState(mockDocumentStore, "p", "\"" + STRING_HUGEVALUE + "\"", mockCompression); | |||
|
|
|||
| assertEquals(documentPropertyState.getValue(Type.STRING), STRING_HUGEVALUE); | |||
| assertEquals(STRING_HUGEVALUE, documentPropertyState.getValue(Type.STRING)); | |||
There was a problem hiding this comment.
swapped expected & actual values to put them in correct order.
| @@ -109,7 +114,7 @@ public void stringAboveThresholdSize() { | |||
|
|
|||
| assertEquals(result.length, STRING_HUGEVALUE.length() + 2 ); | |||
|
|
|||
| assertEquals(state.getValue(Type.STRING), STRING_HUGEVALUE); | |||
| assertEquals(STRING_HUGEVALUE, state.getValue(Type.STRING)); | |||
There was a problem hiding this comment.
swapped expected & actual values to put them in correct order.
| @@ -125,7 +130,7 @@ public void uncompressValueThrowsException() throws IOException { | |||
| CompressedDocumentPropertyState.setCompressionThreshold(DEFAULT_COMPRESSION_THRESHOLD); | |||
| PropertyState documentPropertyState = DocumentPropertyStateFactory.createPropertyState(mockDocumentStore, "p", STRING_HUGEVALUE, mockCompression); | |||
|
|
|||
| assertEquals(documentPropertyState.getValue(Type.STRING), "{}"); | |||
| assertEquals("{}", documentPropertyState.getValue(Type.STRING)); | |||
There was a problem hiding this comment.
swapped expected & actual values to put them in correct order.
| @@ -143,11 +148,10 @@ public void testInterestingStringsWithoutCompression() { | |||
| } | |||
|
|
|||
| @Test | |||
| public void testOneCompressOtherUncompressInEquals() throws IOException { | |||
| public void testOneCompressOtherUncompressInEquals() { | |||
There was a problem hiding this comment.
Removed Un-necessary throws
| } | ||
|
|
||
|
|
||
| @Test | ||
| public void testEqualsWithCompression() throws IOException { | ||
| public void testEqualsWithCompression() { |
There was a problem hiding this comment.
Removed Un-necessary throws
| @Before | ||
| public void before() throws Exception { | ||
| public void before() { |
There was a problem hiding this comment.
Removed Un-necessary throws
There was a problem hiding this comment.
I'd prefer to make the test extend AbstractDocumentStoreTest and so align it with how other tests that are supposed to run with multiple/specific persistences; see proposal in 009d68e.
The way it is written now is that nsfixtures is ignored, and tests with other flavors of RDB will never be executed. That's why we have a test framework that should be used.
The other changes in test details are good, and I'll be happy to incorporate them into my proposal (see #1751).
| @@ -59,7 +59,7 @@ public DocumentMK.Builder getBuilder() { | |||
| } | |||
|
|
|||
| @Before | |||
| public void startUp() throws Exception { | |||
| public void startUp() { | |||
There was a problem hiding this comment.
Removed Un-necessary throws
|
Please include the changes fromhttps://github.com//pull/1751/commits/e1e7c10a45c24e5998ae6562e23d0ea740bdf969 (Utils has a name mapper), and remove the changes to AbstractDocumentStoreTest (whch I want to update soon anyway). |
…mentPropertyStateTest
rishabhdaim
force-pushed
the
OAK-11113
branch
2 times, most recently
from
4ed8c01
to
641feef
Compare
| } | ||
|
|
||
| store = builderProvider.newBuilder().setDocumentStore(ds).getNodeStore(); | ||
| removeMeClusterNodes.add("" + store.getClusterId()); |
There was a problem hiding this comment.
We required this to avoid recovery from taking place.
|
@rishabhdaim noticed that this wasn't squashed - I'm wondering if there's anything to prevent this from happening |
…yStateTest