Add tests for BulkUpload #2

bryant-jimenez · 2024-04-08T23:45:22Z

UI Testing for BulkUpload

♻️ Current situation & Problem

Link any open issues or pull requests (PRs) related to this PR. Please ensure that all non-trivial PRs are first tracked and discussed in an existing GitHub issue or discussion.

Better Support for Batch Uploads of Historical HealthKit Data StanfordSpezi/SpeziHealthKit#17
Currently there were no UI tests for BulkUpload type, this PR adds some.

⚙️ Release Notes

Add a bullet point list summary of the feature and possible migration guides if this is a breaking change so this section can be added to the release notes.
Include code snippets that provide examples of the feature implemented or links to the documentation if it appends or changes the public interface.

📚 Documentation

Please ensure that you properly document any additions in conformance to Spezi Documentation Guide.
You can use this section to describe your solution, but we encourage contributors to document your reasoning and changes using in-line documentation.

✅ Testing

Please ensure that the PR meets the testing requirements set by CodeCov and that new functionality is appropriately tested.
This section describes important information about the tests and why some elements might not be testable.

📝 Code of Conduct & Contributing Guidelines

By submitting creating this pull request, you agree to follow our Code of Conduct and Contributing Guidelines:

I agree to follow the Code of Conduct and Contributing Guidelines.

mjoerke

Thanks for adding the test cases! Mostly looks good but there are some small things I left comments on

Sources/SpeziHealthKit/BulkUploadConstraint.swift

mjoerke · 2024-04-09T01:42:52Z

Sources/SpeziHealthKit/HealthKit.swift

+            HKObjectType.quantityType(forIdentifier: .stepCount)!,
+            HKObjectType.quantityType(forIdentifier: .activeEnergyBurned)!
+        ]
+        try await healthStore.requestAuthorization(toShare: toShareSampleTypes, read: healthKitSampleTypes)


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

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.

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)

@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! 🙏🏽

@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 🚀

Sources/SpeziHealthKit/BulkUpload/BulkUploadSampleDataSource.swift

mjoerke · 2024-04-09T01:54:22Z

Tests/UITests/TestApp/HealthKitTestsView.swift

+            return
+        }
+        let HKStore = HKHealthStore()
+        let startDate = Date()


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?

mjoerke · 2024-04-09T01:55:59Z

Tests/UITests/TestApp/TestAppDelegate.swift

+                    Set([HKQuantityType(.stepCount)]),
+                    predicate: HKQuery.predicateForSamples(
+                        withStart: Calendar.current.date(byAdding: .month, value: -6, to: .now),
+                        end: nil,


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?

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?

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

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

mjoerke · 2024-04-09T01:57:40Z

Tests/UITests/TestApp/HealthKitTestsView.swift

@@ -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()


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

re other comment about accessing healthstore

Add tests for BulkUpload

c091db4

bryant-jimenez assigned mjoerke and bryant-jimenez Apr 8, 2024

bryant-jimenez requested a review from mjoerke April 8, 2024 23:45

mjoerke requested changes Apr 9, 2024

View reviewed changes

remove unused functions/parameters

acbe175

mjoerke approved these changes Apr 9, 2024

View reviewed changes

bryant-jimenez and others added 4 commits April 9, 2024 14:06

Fix modifying SpeziHealthKit to inject data

19b84bf

fix BulkUploadConstraint documentation

0272c57

Total samples var in query

3fd0c16

Merge branch 'main' into bulkupload

e8b17fa

mjoerke force-pushed the main branch from 62d8a5c to 9a347e4 Compare April 13, 2024 05:29

mjoerke merged commit 5f4fb6e into main Jan 8, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add tests for BulkUpload #2

Add tests for BulkUpload #2

bryant-jimenez commented Apr 8, 2024

mjoerke left a comment

mjoerke Apr 9, 2024

bryant-jimenez Apr 9, 2024 •

edited

Loading

mjoerke Apr 9, 2024

bryant-jimenez Apr 9, 2024

PSchmiedmayer Apr 10, 2024

mjoerke Apr 9, 2024

mjoerke Apr 9, 2024

bryant-jimenez Apr 9, 2024 •

edited

Loading

bryant-jimenez Apr 9, 2024

mjoerke Apr 9, 2024

mjoerke Apr 9, 2024

bryant-jimenez Apr 9, 2024

Add tests for BulkUpload #2

Add tests for BulkUpload #2

Conversation

bryant-jimenez commented Apr 8, 2024

UI Testing for BulkUpload

♻️ Current situation & Problem

⚙️ Release Notes

📚 Documentation

✅ Testing

📝 Code of Conduct & Contributing Guidelines

mjoerke left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bryant-jimenez Apr 9, 2024 • edited Loading

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

Choose a reason for hiding this comment

bryant-jimenez Apr 9, 2024 • edited Loading

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

bryant-jimenez Apr 9, 2024 •

edited

Loading

bryant-jimenez Apr 9, 2024 •

edited

Loading