Skip to content

Commit

Permalink
Address @mdboom's change requests
Browse files Browse the repository at this point in the history
  • Loading branch information
travis79 committed Jan 21, 2020
1 parent 227974d commit 328158d
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 39 deletions.
12 changes: 6 additions & 6 deletions glean-core/ios/Glean/Scheduler/MetricsPingScheduler.swift
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,12 @@ class MetricsPingScheduler {
func isDifferentVersion() -> Bool {
// Determine if the version has changed since the last time we ran.
let currentVersion = AppInfo.displayVersion
if let lastVersion = UserDefaults.standard.string(forKey: Constants.lastVersionOfAppUsed) {
if currentVersion != lastVersion {
UserDefaults.standard.set(currentVersion, forKey: Constants.lastVersionOfAppUsed)
return true
}
let lastVersion = UserDefaults.standard.string(forKey: Constants.lastVersionOfAppUsed)
if currentVersion != lastVersion {
UserDefaults.standard.set(currentVersion, forKey: Constants.lastVersionOfAppUsed)
return true
}

return false
}

Expand Down Expand Up @@ -105,7 +105,7 @@ class MetricsPingScheduler {
// this check at startup (when overduePingAsFirst is true).
if isDifferentVersion() {
Dispatchers.shared.serialOperationQueue.addOperation {
self.collectPingAndReschedule(now)
self.collectPingAndReschedule(now, startupPing: true)
}
return
}
Expand Down
2 changes: 1 addition & 1 deletion glean-core/ios/Glean/Utils/Utils.swift
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ extension Date {
dateFormatter.dateFormat = dateFormatPatterns[precision]
return dateFormatter.string(from: self)
}

/// Overloads the operator so that subtraction between two dates results in a TimeInterval representing
/// the difference between them
static func - (lhs: Date, rhs: Date) -> TimeInterval {
Expand Down
66 changes: 34 additions & 32 deletions glean-core/ios/GleanTests/Scheduler/MetricsPingSchedulerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class MetricsPingSchedulerTests: XCTestCase {
override func setUp() {
Glean.shared.enableTestingMode()
}

override func tearDown() {
expectation = nil
}
Expand Down Expand Up @@ -126,8 +126,8 @@ class MetricsPingSchedulerTests: XCTestCase {
value: 1,
to: now,
wrappingComponents: true
)!
)!
)!
XCTAssertEqual(
fireDate.toISO8601String(precision: .second),
mps.timer?.fireDate.toISO8601String(precision: .second),
Expand Down Expand Up @@ -300,50 +300,52 @@ class MetricsPingSchedulerTests: XCTestCase {
Glean.shared.resetGlean(clearStores: true)
Glean.shared.testDestroyGleanHandle()
}

func testTimerInvocation() {
let mps = Glean.shared.metricsPingScheduler

// Set the last time the "metrics" ping was set to now. This will be updated if
// the timer fires so we can detect the change to determine success
let now = Date()
Glean.shared.metricsPingScheduler.updateSentDate(now)
// Converting to strings here because comparing dates is more difficult
XCTAssertEqual(
now.toISO8601String(precision: .second),
mps.getLastCollectedDate()?.toISO8601String(precision: .second)
)

// Create a fake date/time that is just a few seconds before the 4 AM time so
// that it will fire off after a few seconds.
let fakeNow = Calendar.current.date(
bySettingHour: MetricsPingScheduler.Constants.dueHourOfTheDay - 1,
minute: 59,
second: 55,
of: now
)!

// Calling `schedulePingCollection` with our `fakeNow` should cause the timer to
// be set to fire in @ 5 seconds
mps.schedulePingCollection(fakeNow, sendTheNextCalendarDay: false)

let semaphore = DispatchSemaphore(value: 0)

// Launched off the main thread so we can await the semaphore
DispatchQueue.global().async {

let mps = Glean.shared.metricsPingScheduler

// Set the last time the "metrics" ping was set to now. This will be updated if
// the timer fires so we can detect the change to determine success
let now = Date()
Glean.shared.metricsPingScheduler.updateSentDate(now)
// Converting to strings here because comparing dates is more difficult
XCTAssertEqual(
now.toISO8601String(precision: .second),
mps.getLastCollectedDate()?.toISO8601String(precision: .second)
)

// Create a fake date/time that is just a few seconds before the 4 AM time so
// that it will fire off after a few seconds.
let fakeNow = Calendar.current.date(
bySettingHour: MetricsPingScheduler.Constants.dueHourOfTheDay - 1,
minute: 59,
second: 55,
of: now
)!

// Calling `schedulePingCollection` with our `fakeNow` should cause the timer to
// be set to fire in @ 5 seconds
mps.schedulePingCollection(fakeNow, sendTheNextCalendarDay: false)


// Loop and wait for the last collected date to be updated when the timer
// fires
var same = true
repeat {
let nowStr = now.toISO8601String(precision: .second)
let mpsStr = mps.getLastCollectedDate()!.toISO8601String(precision: .second)
same = nowStr.compare(mpsStr) == .orderedSame
} while (same)
} while same

// The date was updated so we can signal the semaphore
semaphore.signal()
}

// Wait up to 10 seconds for the semaphore to be signalled
if semaphore.wait(timeout: .now() + 10.0) == .timedOut {
XCTFail("Timed out waiting for timer to fire")
Expand Down

0 comments on commit 328158d

Please sign in to comment.