-
Notifications
You must be signed in to change notification settings - Fork 243
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
Some fixes for view duration and added tests #347
Conversation
CountlyTests/CountlyViewTests.swift
Outdated
wait(for: [pauseViewExpectation, resumeViewExpectation], timeout: 10) | ||
} | ||
|
||
func testViewTrackingWithBackgroundAndForegroundNotifications() throws { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addition of test comments would be great
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a nice addition of test suite.
It might be extended by scenarios
For example what happens after consent removal etc.
CountlyTests/CountlyViewTests.swift
Outdated
} | ||
|
||
wait(for: [expectation], timeout: 5.0) // Wait for the expectation to be fulfilled | ||
checkRecordedEventsForView(viewName: "View1") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you may also want to validate duration of the views
CountlyTests/CountlyViewTests.swift
Outdated
resumeViewExpectation.fulfill() | ||
} | ||
wait(for: [expectation], timeout: 5.0) // Wait for the expectation to be fulfilled | ||
checkRecordedEventsForView(viewName: "View1", segmentation: ["key": "value"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for example here. View must have a duration of 4 secondo
CountlyTests/CountlyViewTests.swift
Outdated
NotificationCenter.default.post(name: UIApplication.didBecomeActiveNotification, object: nil) | ||
fgExpectation.fulfill() | ||
wait(for: [expectation], timeout: 5.0) // Wait for the expectation to be fulfilled | ||
checkRecordedEventsForView(withID: viewID) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we want to see specifically also those durations are validated and SDK correctly calculates the duration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool work! looks all good to me. just couldn't be sure whether we validate each param under a view request or not but I don't think there's anything wrong with the current way
No description provided.