Skip to content
This repository was archived by the owner on Feb 24, 2025. It is now read-only.

Commit 0fe6554

Browse files
authored
Improve unit tests reliability (#3430)
<!-- Note: This checklist is a reminder of our shared engineering expectations. Feel free to change it, although assigning a GitHub reviewer and the items in bold are required. ⚠️ If you're an external contributor, please file an issue first before working on a PR, as we can't guarantee that we will accept your changes if they haven't been discussed ahead of time. Thanks! --> Task/Issue URL: https://app.asana.com/0/414709148257752/1208526755021935/f Tech Design URL: CC: **Description**: Improve reliability of test suite. <!-- If at any point it isn't actively being worked on/ready for review/otherwise moving forward strongly consider closing it (or not opening it in the first place). If you decide not to close it, use Draft PR while work is still in progress or use `DO NOT MERGE` label to clarify the PRs state and comment with more information. --> **Steps to test this PR**: Validate unit test changes and whether refactoring had any impact on actual code and production flows. <!-- Before submitting a PR, please ensure you have tested the combinations you expect the reviewer to test, then delete configurations you *know* do not need explicit testing. Using a simulator where a physical device is unavailable is acceptable. --> **Definition of Done (Internal Only)**: * [ ] Does this PR satisfy our [Definition of Done](https://app.asana.com/0/1202500774821704/1207634633537039/f)? --- ###### Internal references: [Software Engineering Expectations](https://app.asana.com/0/59792373528535/199064865822552) [Technical Design Template](https://app.asana.com/0/59792373528535/184709971311943)
1 parent 698f13f commit 0fe6554

13 files changed

+289
-158
lines changed

Core/DailyPixel.swift

+20-15
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
//
1919

2020
import Foundation
21+
import Persistence
2122

2223
/// A variant of pixel that is fired at most once per day.
2324
///
@@ -52,6 +53,8 @@ public final class DailyPixel {
5253
error: Swift.Error? = nil,
5354
withAdditionalParameters params: [String: String] = [:],
5455
includedParameters: [Pixel.QueryParameters] = [.appVersion],
56+
pixelFiring: PixelFiring.Type = Pixel.self,
57+
dailyPixelStore: KeyValueStoring = DailyPixel.storage,
5558
onComplete: @escaping (Swift.Error?) -> Void = { _ in }) {
5659
var key: String = pixel.name
5760

@@ -61,13 +64,13 @@ public final class DailyPixel {
6164
key.append(":\(createSortedStringOfValues(from: errorParams))")
6265
}
6366

64-
if !hasBeenFiredToday(forKey: key, dailyPixelStorage: storage) {
65-
Pixel.fire(pixel: pixel,
66-
error: error,
67-
includedParameters: includedParameters,
68-
withAdditionalParameters: params,
69-
onComplete: onComplete)
70-
updatePixelLastFireDate(forKey: key)
67+
if !hasBeenFiredToday(forKey: key, dailyPixelStore: dailyPixelStore) {
68+
pixelFiring.fire(pixel: pixel,
69+
error: error,
70+
includedParameters: includedParameters,
71+
withAdditionalParameters: params,
72+
onComplete: onComplete)
73+
updatePixelLastFireDate(forKey: key, dailyPixelStore: dailyPixelStore)
7174
} else {
7275
onComplete(Error.alreadyFired)
7376
}
@@ -80,12 +83,14 @@ public final class DailyPixel {
8083
error: Swift.Error? = nil,
8184
withAdditionalParameters params: [String: String] = [:],
8285
includedParameters: [Pixel.QueryParameters] = [.appVersion],
86+
pixelFiring: PixelFiring.Type = Pixel.self,
87+
dailyPixelStore: KeyValueStoring = DailyPixel.storage,
8388
onDailyComplete: @escaping (Swift.Error?) -> Void = { _ in },
8489
onCountComplete: @escaping (Swift.Error?) -> Void = { _ in }) {
8590
let key: String = pixel.name
8691

87-
if !hasBeenFiredToday(forKey: key, dailyPixelStorage: storage) {
88-
Pixel.fire(
92+
if !hasBeenFiredToday(forKey: key, dailyPixelStore: dailyPixelStore) {
93+
pixelFiring.fire(
8994
pixelNamed: pixel.name + "_d",
9095
withAdditionalParameters: params,
9196
includedParameters: includedParameters,
@@ -94,25 +99,25 @@ public final class DailyPixel {
9499
} else {
95100
onDailyComplete(Error.alreadyFired)
96101
}
97-
updatePixelLastFireDate(forKey: key)
102+
updatePixelLastFireDate(forKey: key, dailyPixelStore: dailyPixelStore)
98103
var newParams = params
99104
if let error {
100105
newParams.appendErrorPixelParams(error: error)
101106
}
102-
Pixel.fire(
107+
pixelFiring.fire(
103108
pixelNamed: pixel.name + "_c",
104109
withAdditionalParameters: newParams,
105110
includedParameters: includedParameters,
106111
onComplete: onCountComplete
107112
)
108113
}
109114

110-
private static func updatePixelLastFireDate(forKey key: String) {
111-
storage.set(Date(), forKey: key)
115+
private static func updatePixelLastFireDate(forKey key: String, dailyPixelStore: KeyValueStoring) {
116+
dailyPixelStore.set(Date(), forKey: key)
112117
}
113118

114-
private static func hasBeenFiredToday(forKey key: String, dailyPixelStorage: UserDefaults) -> Bool {
115-
if let lastFireDate = dailyPixelStorage.object(forKey: key) as? Date {
119+
private static func hasBeenFiredToday(forKey key: String, dailyPixelStore: KeyValueStoring) -> Bool {
120+
if let lastFireDate = dailyPixelStore.object(forKey: key) as? Date {
116121
return Date().isSameDay(lastFireDate)
117122
}
118123
return false

Core/PixelFiring.swift

+25-3
Original file line numberDiff line numberDiff line change
@@ -27,24 +27,46 @@ public protocol PixelFiring {
2727
includedParameters: [Pixel.QueryParameters],
2828
onComplete: @escaping (Error?) -> Void)
2929

30+
static func fire(pixel: Pixel.Event,
31+
error: Error?,
32+
includedParameters: [Pixel.QueryParameters],
33+
withAdditionalParameters params: [String: String],
34+
onComplete: @escaping (Error?) -> Void)
35+
3036
static func fire(_ pixel: Pixel.Event,
3137
withAdditionalParameters params: [String: String])
38+
39+
static func fire(pixelNamed pixelName: String,
40+
withAdditionalParameters params: [String: String],
41+
includedParameters: [Pixel.QueryParameters],
42+
onComplete: @escaping (Error?) -> Void)
3243
}
3344

3445
extension Pixel: PixelFiring {
3546
public static func fire(_ pixel: Pixel.Event,
3647
withAdditionalParameters params: [String: String],
3748
includedParameters: [Pixel.QueryParameters],
3849
onComplete: @escaping (Error?) -> Void) {
39-
40-
Pixel.fire(pixel: pixel,
50+
51+
Self.fire(pixel: pixel,
4152
withAdditionalParameters: params,
4253
includedParameters: includedParameters,
4354
onComplete: onComplete)
4455
}
4556

4657
public static func fire(_ pixel: Pixel.Event,
4758
withAdditionalParameters params: [String: String]) {
48-
Pixel.fire(pixel: pixel, withAdditionalParameters: params)
59+
Self.fire(pixel: pixel, withAdditionalParameters: params)
60+
}
61+
62+
public static func fire(pixelNamed pixelName: String,
63+
withAdditionalParameters params: [String: String],
64+
includedParameters: [Pixel.QueryParameters],
65+
onComplete: @escaping (Error?) -> Void) {
66+
Self.fire(pixelNamed: pixelName,
67+
withAdditionalParameters: params,
68+
allowedQueryReservedCharacters: nil,
69+
includedParameters: includedParameters,
70+
onComplete: onComplete)
4971
}
5072
}

DuckDuckGoTests/AdAttributionPixelReporterTests.swift

+5-5
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ final class AdAttributionPixelReporterTests: XCTestCase {
4444

4545
let result = await sut.reportAttributionIfNeeded()
4646

47-
XCTAssertEqual(PixelFiringMock.lastPixel, .appleAdAttribution)
47+
XCTAssertEqual(PixelFiringMock.lastPixelName, Pixel.Event.appleAdAttribution.name)
4848
XCTAssertTrue(result)
4949
}
5050

@@ -55,7 +55,7 @@ final class AdAttributionPixelReporterTests: XCTestCase {
5555
await fetcherStorage.markAttributionReportSuccessful()
5656
let result = await sut.reportAttributionIfNeeded()
5757

58-
XCTAssertNil(PixelFiringMock.lastPixel)
58+
XCTAssertNil(PixelFiringMock.lastPixelName)
5959
XCTAssertFalse(result)
6060
}
6161

@@ -65,7 +65,7 @@ final class AdAttributionPixelReporterTests: XCTestCase {
6565

6666
let result = await sut.reportAttributionIfNeeded()
6767

68-
XCTAssertEqual(PixelFiringMock.lastPixel?.name, "m_apple-ad-attribution")
68+
XCTAssertEqual(PixelFiringMock.lastPixelName, "m_apple-ad-attribution")
6969
XCTAssertTrue(result)
7070
}
7171

@@ -130,7 +130,7 @@ final class AdAttributionPixelReporterTests: XCTestCase {
130130

131131
let result = await sut.reportAttributionIfNeeded()
132132

133-
XCTAssertNil(PixelFiringMock.lastPixel)
133+
XCTAssertNil(PixelFiringMock.lastPixelName)
134134
XCTAssertTrue(fetcherStorage.wasAttributionReportSuccessful)
135135
XCTAssertTrue(result)
136136
}
@@ -141,7 +141,7 @@ final class AdAttributionPixelReporterTests: XCTestCase {
141141

142142
let result = await sut.reportAttributionIfNeeded()
143143

144-
XCTAssertNil(PixelFiringMock.lastPixel)
144+
XCTAssertNil(PixelFiringMock.lastPixelName)
145145
XCTAssertFalse(fetcherStorage.wasAttributionReportSuccessful)
146146
XCTAssertFalse(result)
147147
}

0 commit comments

Comments
 (0)