CBL-5668: Get Code Coverage at least 80%

couchbase / couchbase-lite-ios

Lightweight, embedded, syncable NoSQL database engine for iOS and MacOS apps.

Apache License 2.0

1.62k stars 297 forks source link

CBL-5668: Get Code Coverage at least 80% #3289

Closed velicuvlad closed 3 months ago

velicuvlad commented 3 months ago

add a couple of tests for some API methods that weren't already tested.
update LiteCore to fix orphan commit
update VS ext

This is what I've found to be the fastest way to increase the code coverage. There's a big portion of MessageEndpoint class in Swift that is not tested at all but I have to give it time to understand how that works and how to properly test it as there's some setup from what I could grasp from the Obj-C code lines.

This should get Swift to 80% or very very close. It takes a good amount of time to generate the whole release zip on local for all platforms so that it generates the code coverage as well. Hence, if needed, I will follow up with another PR after the new API impl as there will be new tests there as well.

velicuvlad commented 3 months ago

2 tests failing - no changes related to these tests

one of them is some flakiness that Pasin is looking at
the other one Test Case '-[DatabaseTest testConflictHandlerWhenDocumentIsPurged]' started. **** LITECORE OBJECTS STILL NOT FREED: **** /Users/runner/work/couchbase-lite-ios/couchbase-lite-ios/Objective-C/Tests/CBLTestCase.m:88: error: -[DatabaseTest testConflictHandlerWhenDocumentIsPurged] : failed - -4 LiteCore objects have not been freed (see above) Test Case '-[DatabaseTest testConflictHandlerWhenDocumentIsPurged]' failed (2.193 seconds).

I've seen this multiple times ... I have tried multiple times to repro this on local but no luck. Due to the fact that I've seen this multiple times on multiple tests... there might be indeed some mem leak but until I can't prove it for now

velicuvlad commented 3 months ago

No, there's no ticket to port this for Boron yet.

I consider this a blocker for GA as we decided to have at least 80% coverage before leaving the house... especially in this case because is a regression due to the fact that I have removed duplicated test for DB api/moved them to use defaultCollection a while ago. As the intent is to deprecate DB API, there is no point to add them back, but instead to add tests for the things we actually dont test for - hence this PR.

bmeike commented 3 months ago

Right. I completely agree. These are tests for exactly the key feature of the 3.2 release. Do it.

We know we are going to have to do the port to Boron. Might as well open that ticket.

velicuvlad commented 3 months ago

I've made a couple of other modifications. I figured that populateData() uses setValue() for all types. All Document getters tests do use populateData() and then type specific getters. Thus, for the setters tests, I changed everything to type specific setters.