[Ep-248] Page Viewed (search) #1396
Conversation
# Conflicts: # Library/Tracking/KSRAnalytics.swift # Library/Tracking/KSRAnalyticsTests.swift
* Write some tests to tests first visit to the page and subsequent visit after a search has been performed * Fix broken tests after removing old data lake event
Codecov Report
@@ Coverage Diff @@
## master #1396 +/- ##
==========================================
+ Coverage 86.07% 86.08% +0.01%
==========================================
Files 1104 1104
Lines 97608 97707 +99
==========================================
+ Hits 84013 84112 +99
Misses 13595 13595
Continue to review full report at Codecov.
|
* Fix failing tests in SearchViewModelTests
…ty that takes an initial value
| @@ -43,6 +43,7 @@ public func paginate<Cursor, Value: Equatable, Envelope, ErrorEnvelope, RequestP | |||
| skipRepeats: Bool = true, | |||
| valuesFromEnvelope: @escaping ((Envelope) -> [Value]), | |||
| cursorFromEnvelope: @escaping ((Envelope) -> Cursor), | |||
| statsFromEnvelope: ((Envelope) -> Int)? = nil, | |||
There was a problem hiding this comment.
Document the parameter above this
There was a problem hiding this comment.
Should this be marked @escaping like the rest of the parameters, given it's value relies on the network request?
There was a problem hiding this comment.
I learnt that optional closures are already escaping, the compiler shows a warning when an optional closure is marked with @escaping. More info here
Library/PaginateTests.swift
Outdated
| @@ -11,12 +11,13 @@ final class PaginateTests: TestCase { | |||
| let requestFromCursor: (Int) -> SignalProducer<[Int], Never> = { c in .init(value: c <= 2 ? [c] : []) } | |||
| let valuesFromEnvelope: ([Int]) -> [Int] = id | |||
| let cursorFromEnvelope: ([Int]) -> Int = { ($0.last ?? 0) + 1 } | |||
| let statsFromEnvelope: ([Int]) -> Int = { _ in return 10 } | |||
There was a problem hiding this comment.
Why are you returning 10? This should probably be $0.count?
There was a problem hiding this comment.
10 is an arbitrary number I chose. Returning $0.count would return the count of results for a page during pagination, but stats is supposed to be the same regardless of the requested page.
|
|
||
| let searchResults = Signal.merge( | ||
| stats, | ||
| viewWillAppearNotAnimated.mapConst(0).take(first: 1) |
There was a problem hiding this comment.
Can you add a brief inline comment here explaining why this needed to be done
| } | ||
|
|
||
| func testSearchPageViewed_ReturningAfterSearching() { | ||
| let searchProjects = (1...10).map { idx in .template |> Project.lens.id .~ idx } |
There was a problem hiding this comment.
I think you can delete this line and on the next line do let searchResponse = DisoveryEnvelope.template without the rest of the line and the test will have the same desired effect.
* Modify SearchViewModel test to use DiscoveryEnvelope template
# Conflicts: # Library/Tracking/KSRAnalyticsTests.swift
📲 What
Replace Search Page Viewed with a new event. Modify its tracking methods and unit tests.
🤔 Why
Convert Data Lake event to Segment