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.

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

Fuzzy search results on specs list are not what is expected #21879

Closed
sync-by-unito bot opened this issue May 30, 2022 · 8 comments · Fixed by #23325
Closed

Fuzzy search results on specs list are not what is expected #21879

sync-by-unito bot opened this issue May 30, 2022 · 8 comments · Fixed by #23325
Assignees

Comments

@sync-by-unito
Copy link

sync-by-unito bot commented May 30, 2022

Current Behavior

The fuzzy search in the specs list makes absolutely no sense for what I’m actually looking for.

If I search for ‘test’, the result brings up net_work_r_e_que_sts.cy.js.

I would never want this or expect this in my search and it muddies the results.

Screen Shot 2022-05-02 at 9 38 30 AM

Desired Behavior

  • We should use the same search algorithm we had in 9.x as it was fine and no one opened issues about it and you could always find your specs.

Reproducible Steps

  1. Scaffold kitchensink files
  2. Search for ‘test

┆Issue is synchronized with this Jira Bug by Unito
┆Attachments: Screen Shot 2022-05-02 at 9.38.30 AM.png
┆author: Jennifer Shehane
┆epic: 9.x Parity
┆friendlyId: UNIFY-1702
┆priority: High
┆sprint: Unification Release Backlog
┆taskType: Bug

@lmiller1990
Copy link
Contributor

Right, I see the problem. The difference is VS Code and IDEs will actually float the best match to the top - we don’t do that.

This issue says:

If I search for 'test', the result brings up net_work_r_e_que_sts.cy.js.
I would never want this or expect this in my search and it muddies the results.

This is incorrect - it should show up, because it matches, but since it’s a bad match, it would be at the bottom. IDEs not only filter, the sort. We don't sort - we just filter, at least right now.

I really want this to just sort them, too. At least for me, I’d rather ask for what I want than click around and find what I want - this is why fuzzy searching is better than file trees in IDEs, in general, when you are looking for something.
I’ll leave this note in the issue too, I guess we need more thought before we make any changes

@lmiller1990
Copy link
Contributor

lmiller1990 commented Jun 14, 2022

To clarify, here's how 9.x worked: https://github.com/cypress-io/cypress/blob/v9.7.0/packages/desktop-gui/src/specs/specs-store.js#L45-L58

It’s basically just “includes” without file separators. So, it’s a completely different feature - it’s not fuzzy. The 10.x one actually works the same, if you type in the same search.

The confusion comes from the fact it’s a fuzzy search, but it’s not the one most developers are used to. fuzzy sort really implies two things - fuzzy filter AND sort. Currently, we are just filtering - we are fuzzy without the sort, which as it turns out, is not really what developers have come to expect when looking for files in a project.

Regexp search has been requested a lot, so this is obviously something users are interested in - I think we'd need to drop the file tree view if we want to do this with fuzzy/regexp based searching, otherwise you get the results described in this issue. You cannot SORT and have a file tree, since a file tree is, by nature, sorted already.

@xeger
Copy link

xeger commented Jul 15, 2022

Substring matching seems to be partly broken for me, in a disruptive way.

Here is a spec that has failed in CI. I want to reproduce the failure locally, so I copy a fraction of its path to my clipboard:

image

When I paste it into the quick search, lo, Cypress claims it doesn't exist!

image

If I only specify one slash, I do match against the directory:

image

But if I want to find the file in its parent directory -- since many of my specs have the same filename but exist in different directories -- I once again get no results:

image

This is IMHO a disimprovement over v9. Some fuzziness may be nice for some, but please don't sacrifice exact searches in favor of some fuzzy thing that I don't use (and whose semantics aren't documented anyway).

@marktnoonan
Copy link
Contributor

Thanks for the comment @xeger, working on getting this prioritized soon. Matching the full file path will be tracked separately in #22463: I agree you should be able to just pop the path to a spec in the search field and get what you expect.

To @lmiller1990's point, fuzzy search is great when the best matches appears at the top of a sorted list. I don't think we should abandon the idea of fuzzy search in Cypress, but at least for the treeview, the 9.x search produces far fewer results for most searches, and so it is much easier to find what you are looking for in the result set.

@lmiller1990
Copy link
Contributor

lmiller1990 commented Jul 18, 2022

Yep. I think we should be careful of the technical jargon - the reason the results are so janky now is we do not do a fuzzy search. Even if we did, they wouldn't be ideal, since fuzzy searching is generally not good - what you really want when you use a fuzzy matching algorithm is a fuzzy sort.

@marktnoonan marktnoonan self-assigned this Jul 19, 2022
@marktnoonan marktnoonan removed their assignment Aug 10, 2022
@marktnoonan
Copy link
Contributor

I'm removing my assignment for now, if anybody picks it up, this is the branch that makes the lightweight change of adding the threshold to improve the result set: https://github.com/cypress-io/cypress/tree/marktnoonan/21879

The main thing that will need attention is updating the specs list tests to expect different numbers of results for certain strings, but that otherwise all the search results match. I had planned on refactoring the tests so that these numbers aren't repeated all over the place, but that doesn't have to happen in this PR. The code change itself is tiny.

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Aug 16, 2022

The code for this is done in cypress-io/cypress#23325, but has yet to be released.
We'll update this issue and reference the changelog when it's released.

@tbiethman
Copy link
Contributor

Released in 10.6.0.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v10.6.0, please open a new issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants