Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test Collection API on deleted database #113

Closed
wants to merge 2 commits into from

Conversation

LaurenNguyen14
Copy link
Contributor

Refactor to close db safely

Refactor to close db safely
Copy link
Contributor

@bmeike bmeike left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks really good! Please check:

  • If you create a resource in a test (which may fail!) you have to release it in a finally block
  • Check your try-catch blocks to be sure they are small enough to test only what you want to test.

var changes: MutableList<String>? = null
var latch: CountDownLatch? = null

val token = testCollection.addChangeListener() { c ->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you create a token you have to remove() it in a finally clause....

@@ -239,7 +239,7 @@ public void testSaveThenGetFromAnotherDB() throws CouchbaseLiteException {
assertNotSame(doc1a, doc1b);
assertEquals(doc1a.getId(), doc1b.getId());
assertEquals(doc1a.toMap(), doc1b.toMap());
anotherDb.close();
closeDb(anotherDb);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you need to do this in a finally clause...


anotherDb.close();
closeDb(anotherDb);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here.... finally

try {
String id = "doc_id";
MutableDocument document = new MutableDocument(id);
deleteDb(baseTestDb);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test can fail in VERY misleading ways. The deleteDb could throw a CBLException that is not the right kind. that would cause the test to fail, but for the entirely wrong reason. Move everything that isn't being tested outside the try block. You'll have to add CBLException to the signature.

@LaurenNguyen14
Copy link
Contributor Author

too many conflicts, will open a new PR

@LaurenNguyen14 LaurenNguyen14 deleted the feature/CBL-3540-2 branch August 18, 2022 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants