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

Dedupe strings in tf-idf index & include exactLabelMatch in runCommand in smoke tests #195085

Merged
merged 4 commits into from
Oct 8, 2023

Conversation

TylerLeonhardt
Copy link
Member

@TylerLeonhardt TylerLeonhardt commented Oct 7, 2023

Fixes #194909

This bug was causing things to get a higher score than they deserved.

Also, in order to get the smoke tests to pass this also introduces a new exactLabelMatch property to runCommand.

Full Context

The smoke tests install some extensions and they type workbench.extensions.action.focusExtensionsView into the Command Palette which causes a certain command to show up that puts focus in the extension view so the extensions can be installed.

the runCommand had retry logic and would pick the first result of the quick pick regardless of what it is.
Previously, if you were to type something like a command name (workbench.extensions.action.focusExtensionsView), chances are it wouldn’t match any command except for the one command with that id…if there were no results, it was because the command wasn’t registered yet…so because of the retry logic, that eventually would work as we wait for VS Code to actually register that command.
but with the similar commands feature, now things start to match in cases it didn’t match previously… and in this case, it’s matching things on the first try because those commands are actually registered when the one we’re actually looking for isn’t registered yet.

Before my fix to TF-IDF chunking 'workbench.extensions.action.focusExtensionsView' still matched nothing in TF-IDF (similar commands)… so the tests still pass… but once I tweaked the TF-IDF chunk, suddenly things were matching… and that caused the smoke test to fail.

I had 5 ideas for fixing this:

Smoke Test change ideas:

  • have runCommand use the label of the command instead of the id and wait for exact label match.
  • have the smoke tests wait more before starting
    Command Palette change ideas:
  • have a setting to disable similar commands in the smoke tests because it’s getting in the way
  • introduce a new @id: prefix (similar in the Extension Pane) in the command palette that will look up the command id and do nothing else
    Extension change ideas:
  • extensions stuff needs to be registered more eagerly

This is the first one on the list since it's the most straightforward.

@TylerLeonhardt TylerLeonhardt merged commit bd41b74 into main Oct 8, 2023
19 checks passed
@TylerLeonhardt TylerLeonhardt deleted the tyler/fortunate-cougar branch October 8, 2023 23:47
Alex0007 pushed a commit to Alex0007/vscode that referenced this pull request Oct 26, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Nov 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Similar command feedback
3 participants