Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(20)

Issue 29664565: FavIcon: Fixes #6244 - fixed tests, fixed issue with optional unwraps

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 week ago by a.shephard
Modified:
4 days, 1 hour ago
Reviewers:
d108, dean
Visibility:
Public.

Description

Fixes #6244 - fixed tests, fixed issue with optional unwraps

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+11 lines, -18 lines) Patch
M FavIcon/FavIcon.swift View 1 chunk +5 lines, -6 lines 0 comments Download
M FavIconTests/FavIconsTests.swift View 2 chunks +6 lines, -12 lines 1 comment Download

Messages

Total messages: 3
a.shephard
1 week ago (2018-01-12 10:32:34 UTC) #1
dean
On 2018/01/12 10:32:34, a.shephard wrote: LGTM
4 days, 11 hours ago (2018-01-15 19:29:20 UTC) #2
d108
4 days, 1 hour ago (2018-01-16 04:38:56 UTC) #3
https://codereview.adblockplus.org/29664565/diff/29664566/FavIconTests/FavIco...
File FavIconTests/FavIconsTests.swift (right):

https://codereview.adblockplus.org/29664565/diff/29664566/FavIconTests/FavIco...
FavIconTests/FavIconsTests.swift:66: XCTAssertEqual(1200, firstImage.size.width)
A couple problems here:

1. The 1200x630 image retrieved is not a favicon.
2. Unit tests should not have external dependencies.

There is a tutorial on this at
https://www.natashatherobot.com/unit-testing-swift-dependency-injection/.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5