Suggestions json tests #142
Conversation
…ever get into this state (webview2 will not provide invalid uris to the navigation handlers, it'll fail upstream)
…suggestions, and fixed up the scores to match the Windows implementation. These all pass on Windows now, but unsure how pinned tabs are handled on macOS since Windows doesn't have those yet.
…indows_canonicalization_tests' into alex/suggestions-tests
| "topHits": [], | ||
| "searchSuggestions": [], | ||
| "localSuggestions": [ | ||
| { "type": "openTab", "title": "New Tab", "subtitle": "DuckDuckGo", "score": 153600, "uri": "duck://newtab", "tabId": "f4386ef1-675d-43f0-b0d3-868488ab9c56" }, |
There was a problem hiding this comment.
On the Windows side, we still have this entry as the top local suggestion with the 153600 score. It looks like it's being scored correctly and should be the top local suggestion result. Do you know why it isn't ending up here on macOS?
There was a problem hiding this comment.
It's not shown in open tab suggestions because it's a local New Tab page (duck://newtab) and we only show Settings and Bookmarks local page open tab suggestions
…returned as a tab search suggestion.
|
Added e9127d3 because it was suggesting the tab that was making the request. |
… differently than other de-duping results.
There was a problem hiding this comment.
Are the failing checks related to these changes or are they something we need to update from main to fix?
Also I'm good with the new test data generally at the moment since it's now passing on Windows, but I think we'll either need to update the existing test data or add new test cases to cover the changes Jackson was proposing.
| - `expectation` - object containing: | ||
| - `topHits` - array of expected top hit suggestions (best matches across all sources) | ||
| - `searchSuggestions` - array of expected DuckDuckGo API suggestions | ||
| - `localSuggestions` - array of expected local suggestions (bookmarks, history, tabs, settings pages) |
There was a problem hiding this comment.
Should this also have the platform field added?
…up the matching suggestions to be website instead of phrase.
…from the regression screenshots. I wasn't able to validate the mobile ones, so they may need fixing up.
|
@mallexxx I've pushed more changes to break out Jackson's test cases from https://app.asana.com/0/0/1209660385268480/1209688492719484/f into individual cases and properly tag some search results in the windows generated tests as nav results. The other changes look fine to me, so if these pass for you, I think we can approve this. I'm still working through test failures on Windows in our local unit and integration tests. I think these are all broken because the basic de-duping behavior changed, but I'm still working through them, so it's possible some of this test data is broken or not behaving as intended. Feel free to fix it if so! |
…the displayed title should not include www.
<!-- Note: This template is a reminder of our Engineering Expectations and Definition of Done. Remove sections that don't apply to your changes.⚠️ If you're an external contributor, please file an issue before working on a PR. Discussing your changes beforehand will help ensure they align with our roadmap and that your time is well spent. --> Task/Issue URL: https://app.asana.com/0/1202406491309510/1209499219087615 Privacy Ref Tests PR: duckduckgo/privacy-reference-tests#142
Task URL: https://app.asana.com/0/1202406491309510/1209464112525114/f
apple-browsers PR: 2. Update Suggestions ranking algorithm; add JSON tests apple-browsers#107