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

test-web: test failure in search tests #227248

Closed
aeschli opened this issue Aug 30, 2024 · 12 comments · Fixed by #227256 or microsoft/vscode-test-web#134
Closed

test-web: test failure in search tests #227248

aeschli opened this issue Aug 30, 2024 · 12 comments · Fixed by #227256 or microsoft/vscode-test-web#134
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug search Search widget and operation issues

Comments

@aeschli
Copy link
Contributor

aeschli commented Aug 30, 2024

vscode-test-web helps writing and testing VS Code web extensions and extension tests. It starts a server that serves VS Code Web along with the extension under development.

vscode-test-web contains a sample extension and some sample tests.

One of the samples adds a file system provider that also implements a FileSearchProvider
https://github.com/microsoft/vscode-test-web/blob/3c19e4bc8797cdbacc793f15b7fe0f03acea078d/fs-provider/src/fsProvider.ts#L142

The tests then use it to query a test file system:
https://github.com/microsoft/vscode-test-web/blob/3c19e4bc8797cdbacc793f15b7fe0f03acea078d/sample/src/web/test/suite/search.test.ts#L45

These tests are currently failing with property access on undefined (fqFolderInfo is undefined)

const relativePath = path.posix.relative(fqFolderInfo.folder.path, result.path);

Image

I suspect this was caused by the changes for #214041

Steps

To debug

  • npm run sample-tests
  • in the browser that open, open dev tools and set a breaking point in fileSearchManager.ts line 159
@aeschli aeschli assigned aeschli and andreamah and unassigned aeschli Aug 30, 2024
@andreamah
Copy link
Contributor

Looking into this, this is because the URI that is being returned is not a subfolder of the original workspace folder that is being targeted. I see that we target vscode-test-web://mount/, but the URI that comes back as a result is http://localhost:3000/static/extensions/fs/folder/.bar/.foo. I did not consider this use case of the API. If this is a common use case for web extensions, I will need to make this a candidate.

@andreamah andreamah added candidate Issue identified as probable candidate for fixing in the next release search Search widget and operation issues bug Issue identified by VS Code Team member as probable bug labels Aug 30, 2024
@andreamah andreamah added this to the August 2024 milestone Aug 30, 2024
@andreamah
Copy link
Contributor

I have a fix in #227256, although I don't know whether this is high-severity enough to make it a candidate. Any idea whether it is popular for web extensions to offer results that aren't a subfolder of the original folder option?

@roblourens
Copy link
Member

This is in proposed API, so does it need to be a candidate?

@roblourens
Copy link
Member

Any idea whether it is popular for web extensions to offer results that aren't a subfolder of the original folder option?

I don't know whether other extensions would do this but why does the test do it?

@andreamah
Copy link
Contributor

It's tricky because it's a proposed API that has been around for a long time, so I'm not sure if people are as keen about breakages (esp if they adopted it around its early days)

@andreamah
Copy link
Contributor

When I tested, I actually couldn't get the test to work, but I just ran npm run sample and file search threw the error there also.

@aeschli
Copy link
Contributor Author

aeschli commented Sep 2, 2024

Oh, sorry, I didn't realize that this is a bug in the test search provider.

I make a fix there.
On your side you probably want to throw an proper exception when search provider returns resources from a different file system.
It's not needed for August.

@aeschli
Copy link
Contributor Author

aeschli commented Sep 2, 2024

Sorry for the false alarm, thanks @andreamah for figuring it out.

@andreamah andreamah removed the candidate Issue identified as probable candidate for fixing in the next release label Sep 3, 2024
@andreamah andreamah modified the milestones: August 2024, September 2024 Sep 3, 2024
@andreamah
Copy link
Contributor

@aeschli is it a common case in web to have a URI from outside of the workspace returned as a result? I'm not sure if this is a case that I should support.

@aeschli
Copy link
Contributor Author

aeschli commented Sep 5, 2024

The FileSearchProvider is registered for a certain URI scheme (e.g. registerFileSearchProvider('vscode-test-web', ...) and it's expected to find search results only that in the file system of that scheme. Returning matches in different scheme/filesystem is a bug.

I fixed this in my search provider and it would be good if VS Code checks that and throws a better exception.

That's unrelated to what the workspace is set to.. If the workspace happens to be set to a folder vscode-test-web://foo/bar, then my file system provider will be asked when the user wants to find results in the workspace

@andreamah
Copy link
Contributor

I'm not sure if I understand what you mean by this:

That's unrelated to what the workspace is set to.. If the workspace happens to be set to a folder vscode-test-web://foo/bar, then my file system provider will be asked when the user wants to find results in the workspace

To confirm: are results that you provide using your provider always subfolders of the workspace root URI?

@aeschli
Copy link
Contributor Author

aeschli commented Sep 6, 2024

To confirm: are results that you provide using your provider always subfolders of the workspace root URI?

The FileSearchProvider doesn't care about the current workspace root API. It only looks at the search options ( SearchOption.folder) and it only gives back resources that match the given search options. All results are under the SearchOption.folder.

If the workspace root happens to be on the vscode-test-web file system, then the vscode-test-web-FileSearchProvider will be used with the workspace root URI as SearchOption.folder, and then yes, then, it only returns subfolders of the workspace root URI.

I think right now workspace search is the only user of the FileSearchProvider, but I would expect that, at some point, we also add a workspace.fs.findFiles API that would allow extensions to search in any file system.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue identified by VS Code Team member as probable bug search Search widget and operation issues
Projects
None yet
3 participants