[SSDK-541] Add searchbox validation, change Demo to searchbox #164

aokj4ck · 2024-01-30T17:09:48Z

Description

Fixes SSDK-541
Start adding search-box API validation

Checklist

Update CHANGELOG

…-541

…541-add-searchbox-validation Conflicts: Sources/MapboxSearch/InternalAPI/Engine/ApiType+Core.swift Sources/MapboxSearch/PublicAPI/Engine/ApiType.swift Sources/MapboxSearch/PublicAPI/MapboxSearchVersion.swift Sources/MapboxSearch/PublicAPI/Use Cases/Discover API/Discover.swift

…sts - SSDK-541 - Test mocking needs to support different API engine types to make this easier

aokj4ck · 2024-02-14T20:56:25Z

Depends on #179 which generalizes tests and associates mock data with the API type so that it is clear and easy to understand which API type a test should use (geocoding, SBS, autofill, search-box).

…541-add-searchbox-validation

- Add name prefixes for SBS_ and SearchBox_. - To display in the Xcode UI test classes must coexist in the global namespace so we have to use old-fashioned prefixes. - Add note to reorganize SBS mock data in the future. - Add SearchBoxMockResponse, initially based on SBS, to provide data for search-box API tests. - Fix namespace collision within MockWebServer.setResponse function clobbering response function argument.

aokj4ck · 2024-02-26T15:41:40Z

SearchBox_SearchEngineIntegrationTests.testResolvedSearchResult()
SearchBox_SearchEngineIntegrationTests.testResolvedSearchResultWhenQueryChanged()

will be fixed in SAPI-1176

…tionTypeQuery

…peQuery by correcting mocked delegate behavior

…an Francisco

aokj4ck · 2024-02-26T17:25:54Z

Tests/MapboxSearchUITests/Integration/CategorySuggestionsIntegrationTestCase.swift

@@ -1,16 +1,18 @@
 import XCTest

-class CategorySuggestionsIntegrationTestCase: MockSBSServerUITestCase {
+// Rename to UITestCase


Most of the UI test cases have "Integration" in the name rather than "UI" and this conflicts with the actual integration tests which also have "integration" in their names.

This rename operation and other clean-up changes (split MockResponse enum instances into separate files, rename MockResponse cases) can occur in a separate PR to reduce noise.

aokj4ck · 2024-02-26T17:27:50Z

Tests/MapboxSearchUITests/Integration/FavoritesIntegrationTestCase.swift

@@ -1,6 +1,7 @@
 import XCTest

-class FavoritesIntegrationTestCase: MockSBSServerUITestCase {
+// Rename to UITestCase
+class FavoritesIntegrationTestCase: MockSearchBoxUITestCase {


Favorites UI tests are failing on Minsk name in results and this will be fixed in SAPI-1176

aokj4ck · 2024-02-26T21:40:47Z

Tests/MapboxSearchIntegrationTests/search-box/SearchBox_SearchEngineIntegrationTests.swift

+    func testSuggestionTypeQuery() throws {
+        try server.setResponse(.recursion)
+
+        let updateExpectation = delegate.updateExpectation
+        searchEngine.query = "Recursion"
+        wait(for: [updateExpectation], timeout: 10)
+
+        let firstSuggestion = try XCTUnwrap(searchEngine.suggestions.first)
+        searchEngine.select(suggestion: firstSuggestion)
+
+        /// Mock that a subsequent retrieve call has been made and succeeds.
+        /// This is not implemented in the stub delegate (it reports success).
+        /// We have validated that the chained operations are connected.
+        let nextUpdateExpectation = delegate.updateExpectation
+        wait(for: [nextUpdateExpectation], timeout: 10)
+        XCTAssertFalse(searchEngine.suggestions.isEmpty)


@azarovalex this test function is passing locally but failing on CircleCI, does anything stand-out as wrong?

Hmm I've just tried running these tests in Xcode as well as make ci-full-test and it failed both times.
Could it be that we can't wait for an expectation that was already fulfilled before? Or maybe expectedFulfillmentCount to 2.

Also some other tests from this file fail for me on aa1b605

Those other tests will be fixed in SAPI-1176

Ah I found that, oddly, the CircleCI tests were hitting the subsequent retrieve request but local tests were jumping right to the fulfilled expectation.

On CircleCI I ran modifications of just this test to triage and captured that in commit b370ae3

MapboxSearchIntegrationTests.SearchBox_SearchEngineIntegrationTests testSuggestionTypeQuery, XCTAssertEqual failed: ("Optional("Search Result resolution failed, Layer:[MapboxServerAPIDataProvider], Name:[Recursion Software]")") is not equal to ("Optional("")") /Users/distiller/project/Tests/MapboxSearchIntegrationTests/search-box/SearchBox_SearchEngineIntegrationTests.swift:193 wait(for: [nextExpect], timeout: 10) XCTAssertEqual(delegate.error?.localizedDescription, "") XCTAssertFalse(searchEngine.suggestions.isEmpty)

Please review when you can!

CircleCI is failing with an error object, narrowing this down ``` MapboxSearchIntegrationTests.SearchBox_SearchEngineIntegrationTests testSuggestionTypeQuery, XCTAssertEqual failed: ("Optional("Search Result resolution failed, Layer:[MapboxServerAPIDataProvider], Name:[Recursion Software]")") is not equal to ("Optional("")") /Users/distiller/project/Tests/MapboxSearchIntegrationTests/search-box/SearchBox_SearchEngineIntegrationTests.swift:193 ``` wait(for: [nextExpect], timeout: 10) XCTAssertEqual(delegate.error?.localizedDescription, "") XCTAssertFalse(searchEngine.suggestions.isEmpty) ``` ```

azarovalex · 2024-02-28T16:38:39Z

Sources/MapboxSearch/PublicAPI/SearchOptions.swift

+
+                if !timeRange.contains(value) {
+                    info(
+                        "search-box API time_deviation must be within 1 minute and 30 minutes (found \(value.value) seconds)"


(just my thoughts)
so if the argument is out of bounds we just silently log and do nothing?
maybe we should force change deviation to be in bounds or log with higher log level, like warning
or maybe fail the request alltogether

azarovalex · 2024-02-28T16:39:18Z

Sources/MapboxSearch/PublicAPI/SearchOptions.swift

+                    unit: UnitDuration.minutes
+                )
+                .converted(to: .seconds)
+                let timeRange = (minimumTime...maximumTime)


we can extract some logic here in a function to reuse it down below

aokj4ck added 9 commits January 30, 2024 11:04

Replace supportSBS binary with expandable ApiType enum - SSDK-544

68be932

Fix PlaceAutocomplete ApiType - SSDK-544

d61124e

Improve default parameters for SBS ApiType and make it default - SSDK…

03f62f5

…-541

Change SearchUI to use searchBox - SSDK-541

44daef9

Change Demo app Discover and PlaceAutocomplete to searchBox - SSDK-541

459a898

Add known validation options for search-box SSDK-541

3dad1be

Accept SDK version bump - SSDK-541

90c9aa2

Change PlaceAutocomplete back to SBS until we can build search-box te…

b485654

…sts - SSDK-541 - Test mocking needs to support different API engine types to make this easier

aokj4ck and others added 4 commits February 15, 2024 09:19

Merge branch 'main' into SSDK-541-add-searchbox-validation

445445f

Bump search version swift

2323ce2

Merge branch 'main' of github.com:mapbox/mapbox-search-ios into SSDK-…

b9cfadb

…541-add-searchbox-validation

aokj4ck added 5 commits February 26, 2024 10:54

Fix JSON format for SearchBox_SearchEngineIntegrationTests.testSugges…

6e2a824

…tionTypeQuery

Finish fixing SearchBox_SearchEngineIntegrationTests.testSuggestionTy…

385d69d

…peQuery by correcting mocked delegate behavior

Fix Categor Search UI tests for search-box API

fc2948c

Fix CategorySuggestionsNavigation with Café, fix Feedback with mock S…

6992770

…an Francisco

Fix search integration UI test

27b8483

aokj4ck commented Feb 26, 2024

View reviewed changes

aokj4ck added 3 commits February 26, 2024 12:34

Disable 6 tests that rely on SAPI-1176

fa75ab1

Restore previous behavior for Search Box testSuggestionTypeQuery

1101633

Nudge CI with empty commit

aa1b605

aokj4ck marked this pull request as ready for review February 26, 2024 18:41

aokj4ck requested review from a team as code owners February 26, 2024 18:53

aokj4ck requested review from azarovalex and a user February 26, 2024 18:53

aokj4ck commented Feb 26, 2024

View reviewed changes

aokj4ck added 2 commits February 27, 2024 16:57

Tryfix switch to successExpectation

80585cd

azarovalex reviewed Feb 28, 2024

View reviewed changes

azarovalex approved these changes Feb 28, 2024

View reviewed changes

aokj4ck merged commit f72f8f8 into main Feb 28, 2024
4 checks passed

aokj4ck deleted the SSDK-541-add-searchbox-validation branch February 28, 2024 16:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SSDK-541] Add searchbox validation, change Demo to searchbox #164

[SSDK-541] Add searchbox validation, change Demo to searchbox #164

aokj4ck commented Jan 30, 2024 •

edited by jira bot

Loading

aokj4ck commented Feb 14, 2024

aokj4ck commented Feb 26, 2024

aokj4ck Feb 26, 2024

aokj4ck Feb 26, 2024

aokj4ck Feb 26, 2024

azarovalex Feb 27, 2024

aokj4ck Feb 27, 2024

aokj4ck Feb 28, 2024

azarovalex Feb 28, 2024 •

edited

Loading

azarovalex Feb 28, 2024

[SSDK-541] Add searchbox validation, change Demo to searchbox #164

[SSDK-541] Add searchbox validation, change Demo to searchbox #164

Conversation

aokj4ck commented Jan 30, 2024 • edited by jira bot Loading

Description

Checklist

aokj4ck commented Feb 14, 2024

aokj4ck commented Feb 26, 2024

aokj4ck Feb 26, 2024

Choose a reason for hiding this comment

aokj4ck Feb 26, 2024

Choose a reason for hiding this comment

aokj4ck Feb 26, 2024

Choose a reason for hiding this comment

azarovalex Feb 27, 2024

Choose a reason for hiding this comment

aokj4ck Feb 27, 2024

Choose a reason for hiding this comment

aokj4ck Feb 28, 2024

Choose a reason for hiding this comment

azarovalex Feb 28, 2024 • edited Loading

Choose a reason for hiding this comment

azarovalex Feb 28, 2024

Choose a reason for hiding this comment

aokj4ck commented Jan 30, 2024 •

edited by jira bot

Loading

azarovalex Feb 28, 2024 •

edited

Loading