Fix version 3.0 beta build #691

cbaker6 · 2023-04-07T01:24:53Z

Note1: The watchOS platform should appear after this PR is merged to the main as the added .spi.yml tells Swift Package Index what scheme to use for building watch.

cbaker6 · 2023-04-07T02:43:27Z

@gavirawson-apple This is ready for review, let me know if you want me to make any adjustments.

cbaker6 · 2023-04-07T13:07:19Z

@gavirawson-apple It looks like there are a couple files missing that SPM can see, but prevents testing. These files are currently not part of the Xcode build which is why it succeeds:

TestCoreDataQueryPublisher.swift - this is in the repo, but not in the Xcode project. Currently can't build with it because it's missing the types below
CoreDataSource, QueryPublisher

CareKitStore/CareKitStoreTests/Structs/TestKnowledgeVector.swift

cbaker6 · 2023-04-07T18:24:43Z

I've also tested these changes locally in Xcode 14.3 and fix all errors/warnings.

CareKit/CareKitTests/TestAnyEventStoreExtensions.swift

CareKit/CareKit/Shared/Synchronization/CareStoreFetchRequestController.swift

cbaker6 · 2023-04-13T13:02:40Z

I think the CI for this PR needs approval to run.

You can track the working build in my local PR: cbaker6#8

CareKitStore/CareKitStore/StoreCoordinator/OCKStoreCoordinator.swift

cbaker6 · 2023-04-19T18:30:37Z

.github/workflows/swift.yml

+        exclude:
+          - destination: 'platform=watchOS\ Simulator,OS=9.1,name=Apple\ Watch\ Series\ 5\ \(40mm\)'
+            scheme: 'CareKitUI'


add this back

cbaker6 · 2023-04-19T18:50:05Z

.spi.yml

+     platform: ios
+   - platform: ios
+     scheme: "CareKit"
+   - platform: watchos
+     scheme: "CareKit"


CareKit/CareKitTests/TestAnyEventStoreExtensions.swift

cbaker6 · 2023-04-19T19:37:11Z

CareKitStore/CareKitStore/Structs/OCKScheduleElement.swift

@@ -88,7 +88,7 @@ public struct OCKScheduleElement: Codable, Equatable {
        }
    }

-    private let calendar: Calendar
+    private static var calendar = Calendar.current


Change to static computed property

CareKitStore/CareKitStoreTests/Streaming/TestCoreDataQueryMonitor.swift

cbaker6 · 2023-04-19T19:48:02Z

OCKCatalog/OCKCatalog.xcodeproj/project.xcworkspace/contents.xcworkspacedata

@@ -0,0 +1,4 @@
+<?xml version="1.0" encoding="UTF-8"?>


remove file

cbaker6 · 2023-04-20T14:14:04Z

CareKitUI/CareKitUITests/TestColorExtension.swift

@@ -57,8 +56,7 @@ class TestColorExtension: XCTestCase {

    func testClamping() {
        let start = UIColor(red: 0, green: 0, blue: 0, alpha: 1)
-        let lightened = start.lightened(10)
+        let lightened = start.lightened(1)


This has a pre-condition where the argument passed to lightened has to be <=1. For some reason it hasn't been failing on the iOS tests, but the watchOS tests caught the error.

I found the issue. The reason was TestColorExtension wasn't included in the XCTest scheme so it never ran when the CI tests the Xcode project. I noticed the issue in SPM after #691 (comment) since SPM auto detects test files instead of depending on the project scheme.

The update to include the test is here: 225341b

cbaker6 · 2023-04-20T14:16:38Z

Package.swift

@@ -4,7 +4,7 @@ import PackageDescription
 let package = Package(
    name: "CareKit",
    defaultLocalization: "en",
-    platforms: [.iOS(.v13), .macOS(.v10_15), .watchOS(.v6)],
+    platforms: [.iOS(.v14), .macOS(.v11), .watchOS(.v7)],


Bumps to minimum iOS 14 along with counterparts so SPM matches the Xcode Project.

cbaker6 · 2023-04-20T14:17:26Z

Package.swift

+            name: "CareKitUITests",
+            dependencies: ["CareKitUI"],
+            path: "CareKitUI/CareKitUITests",
+            exclude: ["Info.plist", "CareKitUI.xctestplan"]),


This test target was missing from SPM so CareKitUITests weren't showing up when the project was opened with Package.swift

cbaker6 · 2023-04-20T14:24:07Z

@gavirawson-apple this is ready for review. You can see the passing tests in the CI here: cbaker6#8

The CI should begin running against this repo once it's approved

cbaker6 · 2023-04-22T20:30:48Z

CareKitStore/CareKitStore/CoreData/Synchronization/OCKStore+Synchronization.swift

+                                // 8. Lock in the changes. If this fails, all
+                                //    merged changes will be rolled back and
+                                //    we'll need to try again later.
+                                try self.context.save()


This bug wasn't easy to find, but was introduced with the changes in #553. The issue is with the last increment to the knowledge vector is never saved in context, causing data to be lost when saving OCK objects using a CareKit card or other view because it doesn't have the most up-to-date clock value.

The fix is just to save after the increment and probably have a heuristic that mentions a save should always occur after an increment at some point before exiting the respective block of code.

The current test cases don't catch this issue because they are testing the stores directly as oppose to using the CareKit cards to save the outcomes.

The example case to replicate the issue is below:

Setup Device1 to connect to a OCKRemoteServer

Add tasks

Save an outcome using a CareKit card and sync Device1 to the OCKRemoteServer

Bring a new Device2 online and sync it to the OCKRemoteServer. Device2 will now reflect everything on Device1

Delete the saved outcome using a CareKit card on Device2. This will trigger a pullRevision, but notice during pushRevisions no new revision records will be found because the CareKit didn't have the correct Knowledge Vector clock value because it wasn't saved in context. Causing Device1 to never receive the deleted outcome, forcing the devices out-of-sync

I've tested the updates using my sample and using OCKWatchConnectivityPeer. Before the fix I was able to get the iPhone and watch out-of-sync. After the updates, the devices remained synchronized.

This can also be done after push revisions is complete, but I'm not sure if it's actually better and requires extra code:

// 4. Detect and resolve any conflicts that exist. The store // is a CRDT, so there aren't any conflicts at the data // layer, but we want to resolve concurrent changes. self.resolveConflicts { error in self.context.perform { do { // 5. Lock in the changes. If this fails, all // merged changes will be rolled back and // we'll need to try again later. try self.context.save() // 6. Local revisions will now include any patches // that were applied during conflict resolution. let localKnowledge = self.context.knowledgeVector let localRevisions = try self.computeRevisions( since: remoteKnowledge ) // 7. Bump knowledge vector to indicate that any // objects created beyond this point in time // are considered unknown to the peer. self.context.knowledgeVector.increment( clockFor: self.context.clockID ) // 8. Push conflict resolutions + local changes to remote remote.pushRevisions( deviceRevisions: localRevisions, deviceKnowledge: localKnowledge) { error in if let error = error { os_log("Failed to push revision. %{private}@", log: .store, type: .error, error as NSError) } self.context.perform { do { // 9. Lock in the changes. If this fails, all // merged changes will be rolled back and // we'll need to try again later. try self.context.save() } catch { self.context.rollback() completion(error) return } // 10. The sync is still considered successful // even if the remote doesn't accept the // push. The next time we sync with it, it // will still have the same knowledge // vector, which means the proper diff will // be generated. completion(nil) } } } catch { self.context.rollback() completion(error) } } }

Nice catch!

gavirawson-apple

Looks amazing Corey, I just have one minor suggestion to consider before we merge away.

CareKitStore/CareKitStore/Structs/OCKScheduleElement.swift

Co-authored-by: Gavi Rawson <51756298+gavirawson-apple@users.noreply.github.com>

cbaker6 · 2023-04-28T17:39:53Z

@gavirawson-apple let me know if you need anything else from me on this PR

gavirawson-apple · 2023-05-02T17:27:15Z

Thanks Corey, everything is set here. We're just going through some process on our end.

cbaker6 added 8 commits April 6, 2023 21:23

Fix CI

8f92969

fix tests in Xcode project

b33f1de

update CI

5f0cf46

don't build FHIR on watchOS

0a313d0

Don't test on watch

a92cb94

remove extra files

f07bfef

Build on Xcode 14.3

7cd56b1

cancel on new commits

30a86e1

remove SPM generated build folder

dde967d

cbaker6 added 2 commits April 7, 2023 12:17

build and pass test on watchOS

8bc33a7

remove comments

83d81e4

cbaker6 commented Apr 7, 2023

View reviewed changes

CareKitStore/CareKitStoreTests/Structs/TestKnowledgeVector.swift Show resolved Hide resolved

cbaker6 added 10 commits April 7, 2023 13:00

test CI on watchOS

f672696

Don't test watchOS on FHIR

741b0f2

revert FHIR tests back to iOS

2f7b8d4

add SPI and point to CareKit docs

3f5e441

update proj to latest Xcode

92a6c02

update proj to latest Xcode

b5952ab

update proj to latest Xcode

3ed479a

update proj to latest Xcode

2fda297

Fix CareKitFHIR xcodeproj build and test on watchOS

1a66556

Use fulfillment in async XCTest in newer Xcode

5d2500b

cbaker6 commented Apr 7, 2023

View reviewed changes

CareKit/CareKitTests/TestAnyEventStoreExtensions.swift Show resolved Hide resolved

cbaker6 added 3 commits April 8, 2023 13:56

Make DailyPageViewController store publicly readable

38be8ce

fix docs

4bf2d81

fix OCKCDPostalAddress CoreData init

863a5e8

cbaker6 closed this Apr 9, 2023

cbaker6 added 2 commits April 12, 2023 10:43

testing single queue for startQuery streaming

ef51eff

revert

708d0ec

cbaker6 commented Apr 12, 2023

View reviewed changes

CareKit/CareKit/Shared/Synchronization/CareStoreFetchRequestController.swift Show resolved Hide resolved

cbaker6 closed this Apr 13, 2023

cbaker6 reopened this Apr 13, 2023

cbaker6 changed the title ~~Fix CI for latest version~~ Fix version 3.0 beta build Apr 13, 2023

cbaker6 closed this Apr 13, 2023

cbaker6 reopened this Apr 13, 2023

cbaker6 added 2 commits April 13, 2023 10:53

Test holding stronger references from stream

636aaae

test if AsyncAlgorithms 0.1.0 fixes reference issues

7839f08

cbaker6 commented Apr 13, 2023

View reviewed changes

CareKitStore/CareKitStore/StoreCoordinator/OCKStoreCoordinator.swift Show resolved Hide resolved

doc nits

38c496d

cbaker6 commented Apr 19, 2023

View reviewed changes

cbaker6 added 3 commits April 20, 2023 09:39

fixes

951d081

fix watchOS CareKitUI tests

5d10809

Switch calendar to computed property

fdfcbf3

cbaker6 commented Apr 20, 2023

View reviewed changes

cbaker6 added 2 commits April 20, 2023 10:33

Add TestColorExtension to CareKitUITests in XC project

225341b

Save second KnowledgeVector increment to prevent data loss

698176a

cbaker6 commented Apr 22, 2023

View reviewed changes

cbaker6 mentioned this pull request Apr 23, 2023

Use CareKit 3.0.0 netreconlab/CareKitSample-ParseCareKit#116

Merged

4 tasks

gavirawson-apple approved these changes Apr 25, 2023

View reviewed changes

CareKitStore/CareKitStore/Structs/OCKScheduleElement.swift Outdated Show resolved Hide resolved

Update CareKitStore/CareKitStore/Structs/OCKScheduleElement.swift

b67404d

Co-authored-by: Gavi Rawson <51756298+gavirawson-apple@users.noreply.github.com>

gavirawson-apple merged commit 2f2af4e into carekit-apple:main May 3, 2023

cbaker6 deleted the fixCI branch June 8, 2023 15:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix version 3.0 beta build #691

Fix version 3.0 beta build #691

cbaker6 commented Apr 7, 2023 •

edited

Loading

cbaker6 commented Apr 7, 2023

cbaker6 commented Apr 7, 2023

cbaker6 commented Apr 7, 2023

cbaker6 commented Apr 13, 2023

cbaker6 Apr 19, 2023

cbaker6 Apr 19, 2023

cbaker6 Apr 19, 2023

cbaker6 Apr 19, 2023

cbaker6 Apr 20, 2023

cbaker6 Apr 20, 2023 •

edited

Loading

cbaker6 Apr 20, 2023

cbaker6 Apr 20, 2023

cbaker6 commented Apr 20, 2023

cbaker6 Apr 22, 2023 •

edited

Loading

cbaker6 Apr 22, 2023 •

edited

Loading

gavirawson-apple Apr 25, 2023

gavirawson-apple left a comment

cbaker6 commented Apr 28, 2023

gavirawson-apple commented May 2, 2023

Fix version 3.0 beta build #691

Fix version 3.0 beta build #691

Conversation

cbaker6 commented Apr 7, 2023 • edited Loading

cbaker6 commented Apr 7, 2023

cbaker6 commented Apr 7, 2023

cbaker6 commented Apr 7, 2023

cbaker6 commented Apr 13, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cbaker6 Apr 20, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cbaker6 commented Apr 20, 2023

cbaker6 Apr 22, 2023 • edited Loading

Choose a reason for hiding this comment

cbaker6 Apr 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gavirawson-apple left a comment

Choose a reason for hiding this comment

cbaker6 commented Apr 28, 2023

gavirawson-apple commented May 2, 2023

cbaker6 commented Apr 7, 2023 •

edited

Loading

cbaker6 Apr 20, 2023 •

edited

Loading

cbaker6 Apr 22, 2023 •

edited

Loading

cbaker6 Apr 22, 2023 •

edited

Loading