Skip to content
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.

Sign up for GitHub

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

Fix broken UI tests #5916

Merged
merged 1 commit into from
Mar 7, 2024
Merged

Fix broken UI tests #5916

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 16 additions & 8 deletions ios/MullvadVPN/View controllers/SelectLocation/AllLocationDataSource.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,11 @@ class AllLocationDataSource: LocationDataSourceProtocol {

return switch location {
case let .country(countryCode):
rootNode.descendantNodeFor(code: countryCode)
case let .city(_, cityCode):
rootNode.descendantNodeFor(code: cityCode)
rootNode.descendantNodeFor(codes: [countryCode])
case let .city(countryCode, cityCode):
rootNode.descendantNodeFor(codes: [countryCode, cityCode])
case let .hostname(_, _, hostCode):
rootNode.descendantNodeFor(code: hostCode)
rootNode.descendantNodeFor(codes: [hostCode])
}
}

Expand All @@ -62,7 +62,7 @@ class AllLocationDataSource: LocationDataSourceProtocol {
case let .country(countryCode):
let countryNode = CountryLocationNode(
name: serverLocation.country,
code: countryCode,
code: LocationNode.combineNodeCodes([countryCode]),
locations: [location]
)

Expand All @@ -72,7 +72,11 @@ class AllLocationDataSource: LocationDataSourceProtocol {
}

case let .city(countryCode, cityCode):
let cityNode = CityLocationNode(name: serverLocation.city, code: cityCode, locations: [location])
let cityNode = CityLocationNode(
name: serverLocation.city,
code: LocationNode.combineNodeCodes([countryCode, cityCode]),
locations: [location]
)

if let countryNode = rootNode.countryFor(code: countryCode),
!countryNode.children.contains(cityNode) {
Expand All @@ -82,10 +86,14 @@ class AllLocationDataSource: LocationDataSourceProtocol {
}

case let .hostname(countryCode, cityCode, hostCode):
let hostNode = HostLocationNode(name: relay.hostname, code: hostCode, locations: [location])
let hostNode = HostLocationNode(
name: relay.hostname,
code: LocationNode.combineNodeCodes([hostCode]),
locations: [location]
)

if let countryNode = rootNode.countryFor(code: countryCode),
let cityNode = countryNode.cityFor(code: cityCode),
let cityNode = countryNode.cityFor(codes: [countryCode, cityCode]),
!cityNode.children.contains(hostNode) {
hostNode.parent = cityNode
cityNode.children.append(hostNode)
Expand Down
18 changes: 9 additions & 9 deletions ios/MullvadVPN/View controllers/SelectLocation/CustomListsDataSource.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,29 +43,29 @@ class CustomListsDataSource: LocationDataSourceProtocol {
// Since LocationCellViewModel partly depends on LocationNode.code for
// equality, each node code needs to be prefixed with the code of its
// parent custom list to uphold this.
node.code = "\(listNode.code)-\(node.code)"
node.code = LocationNode.combineNodeCodes([listNode.code, node.code])
}

return listNode
}
}

func node(by locations: [RelayLocation], for customList: CustomList) -> LocationNode? {
guard let customListNode = nodes.first(where: { $0.name == customList.name })
guard let listNode = nodes.first(where: { $0.name == customList.name })
else { return nil }

if locations.count > 1 {
return customListNode
return listNode
} else {
// Each search for descendant nodes needs the parent custom list node code to be
// prefixed in order to get a match. See comment in reload() above.
return switch locations.first {
case let .country(countryCode):
customListNode.descendantNodeFor(code: "\(customListNode.code)-\(countryCode)")
case let .city(_, cityCode):
customListNode.descendantNodeFor(code: "\(customListNode.code)-\(cityCode)")
listNode.descendantNodeFor(codes: [listNode.code, countryCode])
case let .city(countryCode, cityCode):
listNode.descendantNodeFor(codes: [listNode.code, countryCode, cityCode])
case let .hostname(_, _, hostCode):
customListNode.descendantNodeFor(code: "\(customListNode.code)-\(hostCode)")
listNode.descendantNodeFor(codes: [listNode.code, hostCode])
case .none:
nil
}
Expand All @@ -91,12 +91,12 @@ class CustomListsDataSource: LocationDataSourceProtocol {
case let .city(countryCode, cityCode):
rootNode
.countryFor(code: countryCode)?.copy(withParent: parentNode)
.cityFor(code: cityCode)
.cityFor(codes: [countryCode, cityCode])

case let .hostname(countryCode, cityCode, hostCode):
rootNode
.countryFor(code: countryCode)?.copy(withParent: parentNode)
.cityFor(code: cityCode)?
.cityFor(codes: [countryCode, cityCode])?
.hostFor(code: hostCode)
}
}
Expand Down
2 changes: 1 addition & 1 deletion ios/MullvadVPN/View controllers/SelectLocation/LocationCellFactory.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ final class LocationCellFactory: CellFactoryProtocol {
func configureCell(_ cell: UITableViewCell, item: LocationCellViewModel, indexPath: IndexPath) {
guard let cell = cell as? LocationCell else { return }

cell.accessibilityIdentifier = item.node.name
cell.accessibilityIdentifier = item.node.code
cell.locationLabel.text = item.node.name
cell.showsCollapseControl = !item.node.children.isEmpty
cell.isExpanded = item.node.showsChildren
Expand Down
5 changes: 5 additions & 0 deletions ios/MullvadVPN/View controllers/SelectLocation/LocationCellViewModel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ struct LocationCellViewModel: Hashable {
let node: LocationNode
var indentationLevel = 0

func hash(into hasher: inout Hasher) {
hasher.combine(section)
hasher.combine(node)
}

static func == (lhs: Self, rhs: Self) -> Bool {
lhs.node == rhs.node &&
lhs.section == rhs.section
Expand Down
14 changes: 10 additions & 4 deletions ios/MullvadVPN/View controllers/SelectLocation/LocationNode.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,16 +46,18 @@ extension LocationNode {
self.code == code ? self : children.first(where: { $0.code == code })
}

func cityFor(code: String) -> LocationNode? {
self.code == code ? self : children.first(where: { $0.code == code })
func cityFor(codes: [String]) -> LocationNode? {
let combinedCode = Self.combineNodeCodes(codes)
return self.code == combinedCode ? self : children.first(where: { $0.code == combinedCode })
}

func hostFor(code: String) -> LocationNode? {
self.code == code ? self : children.first(where: { $0.code == code })
}

func descendantNodeFor(code: String) -> LocationNode? {
self.code == code ? self : children.compactMap { $0.descendantNodeFor(code: code) }.first
func descendantNodeFor(codes: [String]) -> LocationNode? {
let combinedCode = Self.combineNodeCodes(codes)
return self.code == combinedCode ? self : children.compactMap { $0.descendantNodeFor(codes: codes) }.first
}

func forEachDescendant(do callback: (LocationNode) -> Void) {
Expand All @@ -71,6 +73,10 @@ extension LocationNode {
parent.forEachAncestor(do: callback)
}
}

static func combineNodeCodes(_ codes: [String]) -> String {
codes.joined(separator: "-")
}
}

extension LocationNode {
Expand Down
18 changes: 9 additions & 9 deletions ios/MullvadVPNTests/Location/AllLocationsDataSourceTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,18 @@ class AllLocationsDataSourceTests: XCTestCase {
let rootNode = RootLocationNode(children: dataSource.nodes)

// Testing a selection.
XCTAssertNotNil(rootNode.descendantNodeFor(code: "se"))
XCTAssertNotNil(rootNode.descendantNodeFor(code: "dal"))
XCTAssertNotNil(rootNode.descendantNodeFor(code: "es1-wireguard"))
XCTAssertNotNil(rootNode.descendantNodeFor(code: "se2-wireguard"))
XCTAssertNotNil(rootNode.descendantNodeFor(codes: ["se"]))
XCTAssertNotNil(rootNode.descendantNodeFor(codes: ["us", "dal"]))
XCTAssertNotNil(rootNode.descendantNodeFor(codes: ["es1-wireguard"]))
XCTAssertNotNil(rootNode.descendantNodeFor(codes: ["se2-wireguard"]))
}

func testSearch() throws {
let nodes = dataSource.search(by: "got")
let rootNode = RootLocationNode(children: nodes)

XCTAssertTrue(rootNode.descendantNodeFor(code: "got")?.isHiddenFromSearch == false)
XCTAssertTrue(rootNode.descendantNodeFor(code: "sto")?.isHiddenFromSearch == true)
XCTAssertTrue(rootNode.descendantNodeFor(codes: ["se", "got"])?.isHiddenFromSearch == false)
XCTAssertTrue(rootNode.descendantNodeFor(codes: ["se", "sto"])?.isHiddenFromSearch == true)
}

func testSearchWithEmptyText() throws {
Expand All @@ -42,15 +42,15 @@ class AllLocationsDataSourceTests: XCTestCase {

func testNodeByLocation() throws {
var nodeByLocation = dataSource.node(by: .country("es"))
var nodeByCode = dataSource.nodes.first?.descendantNodeFor(code: "es")
var nodeByCode = dataSource.nodes.first?.descendantNodeFor(codes: ["es"])
XCTAssertEqual(nodeByLocation, nodeByCode)

nodeByLocation = dataSource.node(by: .city("es", "mad"))
nodeByCode = dataSource.nodes.first?.descendantNodeFor(code: "mad")
nodeByCode = dataSource.nodes.first?.descendantNodeFor(codes: ["es", "mad"])
XCTAssertEqual(nodeByLocation, nodeByCode)

nodeByLocation = dataSource.node(by: .hostname("es", "mad", "es1-wireguard"))
nodeByCode = dataSource.nodes.first?.descendantNodeFor(code: "es1-wireguard")
nodeByCode = dataSource.nodes.first?.descendantNodeFor(codes: ["es1-wireguard"])
XCTAssertEqual(nodeByLocation, nodeByCode)
}
}
Expand Down
16 changes: 8 additions & 8 deletions ios/MullvadVPNTests/Location/CustomListsDataSourceTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,21 +22,21 @@ class CustomListsDataSourceTests: XCTestCase {
let nodes = dataSource.nodes

let netflixNode = try XCTUnwrap(nodes.first(where: { $0.name == "Netflix" }))
XCTAssertNotNil(netflixNode.descendantNodeFor(code: "netflix-es1-wireguard"))
XCTAssertNotNil(netflixNode.descendantNodeFor(code: "netflix-se"))
XCTAssertNotNil(netflixNode.descendantNodeFor(code: "netflix-dal"))
XCTAssertNotNil(netflixNode.descendantNodeFor(codes: ["netflix", "es1-wireguard"]))
XCTAssertNotNil(netflixNode.descendantNodeFor(codes: ["netflix", "se"]))
XCTAssertNotNil(netflixNode.descendantNodeFor(codes: ["netflix", "us", "dal"]))

let youtubeNode = try XCTUnwrap(nodes.first(where: { $0.name == "Youtube" }))
XCTAssertNotNil(youtubeNode.descendantNodeFor(code: "youtube-se2-wireguard"))
XCTAssertNotNil(youtubeNode.descendantNodeFor(code: "youtube-dal"))
XCTAssertNotNil(youtubeNode.descendantNodeFor(codes: ["youtube", "se2-wireguard"]))
XCTAssertNotNil(youtubeNode.descendantNodeFor(codes: ["youtube", "us", "dal"]))
}

func testSearch() throws {
let nodes = dataSource.search(by: "got")
let rootNode = RootLocationNode(children: nodes)

XCTAssertTrue(rootNode.descendantNodeFor(code: "netflix-got")?.isHiddenFromSearch == false)
XCTAssertTrue(rootNode.descendantNodeFor(code: "netflix-sto")?.isHiddenFromSearch == true)
XCTAssertTrue(rootNode.descendantNodeFor(codes: ["netflix", "se", "got"])?.isHiddenFromSearch == false)
XCTAssertTrue(rootNode.descendantNodeFor(codes: ["netflix", "se", "sto"])?.isHiddenFromSearch == true)
}

func testSearchWithEmptyText() throws {
Expand All @@ -51,7 +51,7 @@ class CustomListsDataSourceTests: XCTestCase {

func testNodeByLocations() throws {
let nodeByLocations = dataSource.node(by: [.hostname("es", "mad", "es1-wireguard")], for: customLists.first!)
let nodeByCode = dataSource.nodes.first?.descendantNodeFor(code: "netflix-es1-wireguard")
let nodeByCode = dataSource.nodes.first?.descendantNodeFor(codes: ["netflix", "es1-wireguard"])

XCTAssertEqual(nodeByLocations, nodeByCode)
}
Expand Down
4 changes: 2 additions & 2 deletions ios/MullvadVPNTests/Location/LocationNodeTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,15 +81,15 @@ class LocationNodeTests: XCTestCase {
}

func testFindByCityCode() {
XCTAssertTrue(countryNode.cityFor(code: cityNode.code) == cityNode)
XCTAssertTrue(countryNode.cityFor(codes: [cityNode.code]) == cityNode)
}

func testFindByHostCode() {
XCTAssertTrue(cityNode.hostFor(code: hostNode.code) == hostNode)
}

func testFindDescendantByNodeCode() {
XCTAssertTrue(listNode.descendantNodeFor(code: hostNode.code) == hostNode)
XCTAssertTrue(listNode.descendantNodeFor(codes: [hostNode.code]) == hostNode)
}
}

Expand Down
Loading