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.

Sign up for GitHub

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

Add tests for BulkUpload #2

Merged
merged 6 commits into from
Jan 8, 2025
Merged

Add tests for BulkUpload #2

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c091db4
Add tests for BulkUpload
bryant-jimenez Apr 8, 2024
acbe175
remove unused functions/parameters
bryant-jimenez Apr 9, 2024
19b84bf
Fix modifying SpeziHealthKit to inject data
bryant-jimenez Apr 9, 2024
0272c57
fix BulkUploadConstraint documentation
bryant-jimenez Apr 9, 2024
3fd0c16
Total samples var in query
bryant-jimenez Apr 10, 2024
e8b17fa
Merge branch 'main' into bulkupload
mjoerke Apr 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Sources/SpeziHealthKit/BulkUpload/BulkUpload.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public struct BulkUpload: HealthKitDataSourceDescription {
}

public func dataSources(healthStore: HKHealthStore, standard: any Standard) -> [any HealthKitDataSource] {
// Ensure the 'standard' actually conforms to 'BulkUploadConstraint' to use specific add_bulk function.
// Ensure the 'standard' actually conforms to 'BulkUploadConstraint' to use specific processBulk function.
guard let bulkUploadConstraint = standard as? any BulkUploadConstraint else {
preconditionFailure(
"""
Expand Down
21 changes: 1 addition & 20 deletions Sources/SpeziHealthKit/BulkUpload/BulkUploadSampleDataSource.swift
View file
Open in desktop
bryant-jimenez marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -52,25 +52,6 @@ final class BulkUploadSampleDataSource: HealthKitDataSource {
}
// swiftlint:enable function_default_parameter_at_end


private static func loadDefaultQueryDate(for sampleType: HKSampleType) -> Date {
let defaultPredicateDateUserDefaultsKey = UserDefaults.Keys.bulkUploadDefaultPredicateDatePrefix.appending(sampleType.identifier)
guard let date = UserDefaults.standard.object(forKey: defaultPredicateDateUserDefaultsKey) as? Date else {
// We start date collection at the previous full minute mark to make the
// data collection deterministic to manually entered data in HealthKit.
var components = Calendar.current.dateComponents(in: .current, from: .now)
components.setValue(0, for: .second)
components.setValue(0, for: .nanosecond)
let defaultQueryDate = components.date ?? .now

UserDefaults.standard.set(defaultQueryDate, forKey: defaultPredicateDateUserDefaultsKey)

return defaultQueryDate
}
return date
}


func askedForAuthorization() async {
guard askedForAuthorization(for: sampleType) && !deliverySetting.isManual && !active else {
return
Expand Down Expand Up @@ -136,7 +117,7 @@ final class BulkUploadSampleDataSource: HealthKitDataSource {
limit: bulkSize
)
result = try await anchorDescriptor.result(for: healthStore)
} while (!result.addedSamples.isEmpty) && (!result.deletedObjects.isEmpty)
} while (!result.addedSamples.isEmpty) || (!result.deletedObjects.isEmpty)
}

private func saveAnchor() {
Expand Down
1 change: 1 addition & 0 deletions Sources/SpeziHealthKit/BulkUploadConstraint.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,5 +31,6 @@ public protocol BulkUploadConstraint: Standard {
/// - Parameter samplesAdded: The batch of `HKSample`s that should be added.
/// - Parameter objectsDeleted: The batch of `HKSample`s that were deleted from the HealthStore. Included if needed to account for rate limiting
/// when uploading to a cloud provider.
/// - Parameter bulkSize: The specified size of each batch of samples to be fetched.
func processBulk(samplesAdded: [HKSample], samplesDeleted: [HKDeletedObject]) async
}
7 changes: 5 additions & 2 deletions Sources/SpeziHealthKit/HealthKit.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,11 @@ public final class HealthKit: Module, EnvironmentAccessible, DefaultInitializabl
guard !authorized else {
return
}

try await healthStore.requestAuthorization(toShare: [], read: healthKitSampleTypes)
let toShareSampleTypes: Set<HKSampleType> = [
HKObjectType.quantityType(forIdentifier: .stepCount)!,
HKObjectType.quantityType(forIdentifier: .activeEnergyBurned)!
]
try await healthStore.requestAuthorization(toShare: toShareSampleTypes, read: healthKitSampleTypes)
Copy link
Owner

Choose a reason for hiding this comment

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

We cannot hard code the sample types here – this will request authorization to share step count and active energy burned in all applications that use SpeziHealthKit. We should figure out a way to add this in the UI test application, I think you should just be able to call
healthStore.requestAuthorization(toShare: toShareSampleTypes, read: healthKitSampleTypes)
again and it will ask for the updated share permissions

Copy link
Collaborator Author

@bryant-jimenez bryant-jimenez Apr 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for catching this - definitely don't want that to be the case for all users of SpeziHealthKit. The health store is unfortunately a private variable inside of the healthkit module, so I can't access it directly to call requestAuthorization and pass in the types to share.

Copy link
Owner

Choose a reason for hiding this comment

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

This might be a bit of a hack, but are you able to just initialize and access your own HKHealthStore object from within the UI test app? I just don't think adding additional permissions to the entire SpeziHealthKit package is feasible. The alternative would be to add functionality to SpeziHealthKit that allows you to declare types you want to share (in addition to read permissions)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@PSchmiedmayer Hi Paul, right now we're running into issues in the test app with getting write permissions to inject step count data for testing purposes. I have the button that injects data, but has to do so with a separate instance of the apple HealthStore, since SpeziHealthKit currently doesn't support passing in types to request write access (only read).

The issue with this is that now the health authorization sheet pops up when triggering manual data collection in the test app, which shouldn't be the case, but this separate instance is necessary because of the private protection levels in the HealthKit module for the sample types passed in to healthStore.requestAuthorization().

I had originally modified SpeziHealthKit directly (unknowingly) but pivoted (as @mjoerke suggested above) and attempted this separate HKStore instance to try to get around it. With the modifications to SpeziHealthKit (hardcoded but it showed that it could work), the tests successfully passed regarding bulkupload, but we obviously can't leave those hardcoded write permissions in.

Recognizing the time crunch we're on with starting internal user studies this week, we wanted some guidance on how to go about this since it feels like we're spending time "hacking" the test app. Given the time crunch with internal user studies starting this week, we felt it might be best to start integrating within the Prisma app. Eventually it might be good to have SpeziHealthKit allow for some write permissions as well so that we can use them for the test app, but we did want to get moving on integrating within Prisma.

Would love your thoughts/input on this! 🙏🏽

Copy link
Collaborator

Choose a reason for hiding this comment

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

@bryant-jimenez I would suggest to manually request permissions to get write permissions when you press the button to inject the data. You can do all the operations in an async function based on the button press (e.g. how to get write permissions: https://developer.apple.com/documentation/healthkit/authorizing_access_to_health_data) and within your UI test use the same UI testing code that we use to handle and dismiss the HealthKit permissions sheet ... that might be the cleanest way to handle this 🚀


alreadyRequestedSampleTypes = healthKitSampleTypesIdentifiers

Expand Down
5 changes: 5 additions & 0 deletions Tests/UITests/TestApp/HealthKitStore.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,4 +81,9 @@ class HealthKitStore: Module, DefaultInitializable, EnvironmentAccessible {
let request = UNNotificationRequest(identifier: UUID().uuidString, content: content, trigger: nil)
try? await UNUserNotificationCenter.current().add(request)
}

@MainActor
func processBulk(samplesAdded: [HKSample], samplesDeleted: [HKDeletedObject]) async {
samples.append(contentsOf: samplesAdded)
}
}
5 changes: 4 additions & 1 deletion Tests/UITests/TestApp/HealthKitTestAppStandard.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,12 @@ import SpeziHealthKit


/// An example Standard used for the configuration.
actor HealthKitTestAppStandard: Standard, HealthKitConstraint {
actor HealthKitTestAppStandard: Standard, HealthKitConstraint, BulkUploadConstraint {
@Dependency private var healthKitStore: HealthKitStore

func processBulk(samplesAdded: [HKSample], samplesDeleted: [HKDeletedObject]) async {
await healthKitStore.processBulk(samplesAdded: samplesAdded, samplesDeleted: samplesDeleted)
}

func add(sample: HKSample) async {
await healthKitStore.add(sample: sample)
Expand Down
45 changes: 43 additions & 2 deletions Tests/UITests/TestApp/HealthKitTestsView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,22 +14,25 @@ import SwiftUI
struct HealthKitTestsView: View {
@Environment(HealthKit.self) var healthKitModule
@Environment(HealthKitStore.self) var healthKitStore


var body: some View {
List {
AsyncButton("Ask for authorization") {
try? await healthKitModule.askForAuthorization()
Copy link
Owner

Choose a reason for hiding this comment

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

You will probably have to add the toShare types here when you request authorization

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

re other comment about accessing healthstore

}
.disabled(healthKitModule.authorized)
AsyncButton("Inject Step Count Data") {
await injectStepCountData()
}
AsyncButton("Trigger data source collection") {
await healthKitModule.triggerDataSourceCollection()
}
Section("Collected Samples Since App Launch") {
Section( String(healthKitStore.samples.count) + " Collected Samples Since App Launch") {
ForEach(healthKitStore.samples, id: \.self) { element in
Text(element.sampleType.identifier)
}
}

if !HealthKitStore.collectedSamplesOnly {
Section("Background Persistance Log") {
ForEach(healthKitStore.backgroundPersistance, id: \.self) { element in
Expand All @@ -41,4 +44,42 @@ struct HealthKitTestsView: View {
}
}
}

func injectStepCountData() async {
// Generate sample data
guard let quantityType = HKQuantityType.quantityType(forIdentifier: .stepCount) else {
print("Step count quantity type not available.")
return
}
let HKStore = HKHealthStore()
let startDate = Date()
Copy link
Owner

Choose a reason for hiding this comment

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

If I understand this correctly, it's injected 500 step count samples that start now and end 5 minutes from now.

Why does this get triggered in the bulk upload if it is fetching historical data from 6 months ago up until the time the app was first launched?

let endDate = Calendar.current.date(byAdding: .minute, value: 5, to: startDate)!
for num in 1...500 {
let quantity = HKQuantity(unit: .count(), doubleValue: Double(num)) // Simulated step count
let sample = HKQuantitySample(type: quantityType, quantity: quantity, start: startDate, end: endDate)
try await HKStore.save(sample) {success, error in
if let error = error {
print("Error saving step count: \(error.localizedDescription)")
} else {
print("Step count saved successfully.")
}
}
}
}
}


#if DEBUG
#Preview {
List {
AsyncButton("Ask for authorization") {
}
AsyncButton("Trigger data source collection") {
}
Section("Collected Samples Since App Launch") {
}
}

}

#endif
30 changes: 21 additions & 9 deletions Tests/UITests/TestApp/TestAppDelegate.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,26 +14,38 @@ class TestAppDelegate: SpeziAppDelegate {
override var configuration: Configuration {
Configuration(standard: HealthKitTestAppStandard()) {
HealthKit {
CollectSample( //
CollectSample(
HKQuantityType.electrocardiogramType(),
deliverySetting: .background(.manual)
)
CollectSample( //
HKQuantityType(.stepCount),
deliverySetting: .background(.automatic)
)
CollectSample(
HKQuantityType(.pushCount),
deliverySetting: .anchorQuery(.manual)
)
CollectSample( //
HKQuantityType(.activeEnergyBurned),
deliverySetting: .anchorQuery(.automatic)
)
CollectSample( //
HKQuantityType(.restingHeartRate),
deliverySetting: .manual()
)
BulkUpload(
Set([HKQuantityType(.stepCount)]),
predicate: HKQuery.predicateForSamples(
withStart: Calendar.current.date(byAdding: .month, value: -6, to: .now),
end: nil,
Copy link
Owner

Choose a reason for hiding this comment

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

Re the previous comment about the BulkUpload triggering, I'm not 100% about how end: nil is interpreted, but we might want to have the query end with Date() such that we don't double count samples?

Couldn't find an answer in the documentation but it might mean the predicate includes all incoming samples in the future?

Copy link
Collaborator Author

@bryant-jimenez bryant-jimenez Apr 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Im actually not too sure about this either, but your point makes sense - these shouldnt be collected based on the current predicate. This is the answer I got from GPT:

The provided query predicate is interpreted as follows:

  • HKQuery.predicateForSamples(withStart:end:options:) is a method used to create a predicate for querying HealthKit samples within a specific time range.
  • In this case:
    • withStart parameter specifies the start date of the query's time range. It is calculated by subtracting 6 months from the current date using Calendar.current.date(byAdding: .month, value: -6, to: .now).
    • end parameter is set to nil, indicating that there is no end date specified, meaning the query will continue to the current time.
    • options parameter is set to .strictEndDate, which specifies that the end date is exclusive, meaning samples collected exactly at the end date will not be included in the query results.

So, the predicate will retrieve HealthKit samples collected within the last 6 months up to the current moment, excluding any samples collected exactly at the current time.

Would it be possible that since the samples are uploaded milliseconds (or a very very short time) before the query starts executing, they're still being collected? And since end is nil, it includes these since its technically not the current time?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think specifying Date() should fix it, will give it a try and also modify the samples that I inject

Copy link
Owner

Choose a reason for hiding this comment

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

Thanks for looking into this – not sure I trust GPT's answer 100% since the answer is sparsely documented on the internet, but it could be true. I think it's a good idea for the BulkUpload to end at Date() either way, both in this test app and our Prisma app, since that clear specifies which samples belong in BulkUpload vs CollectSamples

options: .strictEndDate
),
bulkSize: 100,
deliveryStartSetting: .manual
)
BulkUpload(
Set([HKQuantityType(.activeEnergyBurned)]),
predicate: HKQuery.predicateForSamples(
withStart: Calendar.current.date(byAdding: .month, value: -6, to: .now),
end: nil,
options: .strictEndDate
),
bulkSize: 2,
deliveryStartSetting: .automatic
)
}
}
}
Expand Down
42 changes: 42 additions & 0 deletions Tests/UITests/TestAppUITests/SpeziHealthKitTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,48 @@ import XCTHealthKit


final class HealthKitTests: XCTestCase {
func testBulkUpload() throws {
let app = XCUIApplication()
app.launchArguments = ["--collectedSamplesOnly"]
app.deleteAndLaunch(withSpringboardAppName: "TestApp")

try exitAppAndOpenHealth(.activeEnergy)
try exitAppAndOpenHealth(.activeEnergy)
try exitAppAndOpenHealth(.activeEnergy)
try exitAppAndOpenHealth(.activeEnergy)
try exitAppAndOpenHealth(.activeEnergy)

app.activate()
XCTAssert(app.buttons["Ask for authorization"].waitForExistence(timeout: 2))
app.buttons["Ask for authorization"].tap()
try app.handleHealthKitAuthorization()
app.hkTypeIdentifierAssert(
[
.activeEnergy: 5
]
)

XCTAssert(app.buttons["Inject Step Count Data"].waitForExistence(timeout: 2))
app.buttons["Inject Step Count Data"].tap()

XCTAssert(app.buttons["Trigger data source collection"].waitForExistence(timeout: 2))
app.buttons["Trigger data source collection"].tap()

// Define a predicate to match elements with the desired label
let predicate = NSPredicate(format: "label CONTAINS 'Collected Samples Since App Launch'")

// Locate the Section() element containing the text with the number of samples collected
let sectionElement = app.staticTexts.matching(predicate).element
XCTAssert(sectionElement.waitForExistence(timeout: 2))

let labelText = sectionElement.label
// Parse the integer from the label text (starts with integer)
let samplesCount = Int(labelText.split(separator: " ").first ?? "")

XCTAssertEqual(samplesCount, 505)
}


func testHealthKit() throws { // swiftlint:disable:this function_body_length
let app = XCUIApplication()
app.launchArguments = ["--collectedSamplesOnly"]
Expand Down
6 changes: 6 additions & 0 deletions Tests/UITests/UITests.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,9 @@
ENABLE_PREVIEWS = YES;
ENABLE_TESTING_SEARCH_PATHS = YES;
GENERATE_INFOPLIST_FILE = YES;
INFOPLIST_KEY_LSApplicationCategoryType = "";
INFOPLIST_KEY_NSHealthShareUsageDescription = "The TestApp accesses your HealthKit data to run the tests.";
INFOPLIST_KEY_NSHealthUpdateUsageDescription = "The TestApp accesses your HealthKit data to run the tests.";
INFOPLIST_KEY_UIApplicationSceneManifest_Generation = YES;
INFOPLIST_KEY_UIApplicationSupportsIndirectInputEvents = YES;
INFOPLIST_KEY_UILaunchScreen_Generation = YES;
Expand Down Expand Up @@ -447,7 +449,9 @@
ENABLE_PREVIEWS = YES;
ENABLE_TESTING_SEARCH_PATHS = YES;
GENERATE_INFOPLIST_FILE = YES;
INFOPLIST_KEY_LSApplicationCategoryType = "";
INFOPLIST_KEY_NSHealthShareUsageDescription = "The TestApp accesses your HealthKit data to run the tests.";
INFOPLIST_KEY_NSHealthUpdateUsageDescription = "The TestApp accesses your HealthKit data to run the tests.";
INFOPLIST_KEY_UIApplicationSceneManifest_Generation = YES;
INFOPLIST_KEY_UIApplicationSupportsIndirectInputEvents = YES;
INFOPLIST_KEY_UILaunchScreen_Generation = YES;
Expand Down Expand Up @@ -581,7 +585,9 @@
ENABLE_PREVIEWS = YES;
ENABLE_TESTING_SEARCH_PATHS = YES;
GENERATE_INFOPLIST_FILE = YES;
INFOPLIST_KEY_LSApplicationCategoryType = "";
INFOPLIST_KEY_NSHealthShareUsageDescription = "The TestApp accesses your HealthKit data to run the tests.";
INFOPLIST_KEY_NSHealthUpdateUsageDescription = "The TestApp accesses your HealthKit data to run the tests.";
INFOPLIST_KEY_UIApplicationSceneManifest_Generation = YES;
INFOPLIST_KEY_UIApplicationSupportsIndirectInputEvents = YES;
INFOPLIST_KEY_UILaunchScreen_Generation = YES;
Expand Down