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.runTests does not take effect #125712

Closed
jdneo opened this issue Jun 8, 2021 · 8 comments
Closed

test.runTests does not take effect #125712

jdneo opened this issue Jun 8, 2021 · 8 comments
Assignees
Labels
api insiders-released Patch has been released in VS Code Insiders testing Built-in testing support
Milestone

Comments

@jdneo
Copy link
Member

jdneo commented Jun 8, 2021

Version: 1.57.0-insider (user setup)
Commit: bf84ee6
Date: 2021-06-07T12:46:22.208Z
Electron: 12.0.9
Chrome: 89.0.4389.128
Node.js: 14.16.0
V8: 8.9.255.25-electron.0
OS: Windows_NT x64 10.0.19043

Hi @connor4312,

It seems that test.runTests() does not take effect. Does that mean this API is not aiming to finalize for the first stable iteration?

In Java we have existing features that user can trigger tests from other view(e.g. Java Project explorer). I guess this API is for such purpose?

Or can we directly use test.createTestRun(request) to achieve that? If yes, how to get the cancellation token?

@connor4312
Copy link
Member

Yea, my logic for not finalizing it was that it would mostly be called by other extensions. But it's reasonable to want to call it yourself, I'll fix it up and move it for finalization.

@connor4312
Copy link
Member

I don't reproduce this issue: https://memes.peet.io/img/21-06-e8b99a9a-a862-4dfb-92b4-a057b9ed18eb.mp4

Can you add instructions?

@connor4312 connor4312 added info-needed Issue requires more information from poster testing Built-in testing support labels Jun 9, 2021
@jdneo
Copy link
Member Author

jdneo commented Jun 10, 2021

Is it because the test item passed to this API must be the item in the test explorer?

Use the following code can repro the issue:

vscode.commands.registerCommand('test-provider-sample.runTests', async tests => {
      const root = WorkspaceTestRoot.create(vscode.workspace.workspaceFolders![0]);
      await vscode.test.runTests({ tests: [root], debug: false });
      vscode.window.showInformationMessage('Test run complete');
})

More background

In Java we have a explorer called Java Project explorer which provides the package view similar to other Java IDEs.

Meanwhile, there are some 'shortcut' commands registered to that view, like run tests

image

my original attempt is trying to create a new test item for the node in Java Project explorer then pass it to test.runTests()

@connor4312
Copy link
Member

connor4312 commented Jun 10, 2021

Ah, I see. Currently this works only for tests that are "attached" to an existing item, returned from the TestController, and this is tricky to change:

  • Given a free-floating TestItem, we don't know what Controller it's associated with. If your extension registered multiple TestControllers, we'd have no idea which one to invoke.
  • runTests can be called from any extension. We guarantee to the TestController that we will only ask it to run tests that it created, which allows it to depend on the custom data in the test. Other extensions may not fulfill this contract. Even barring that, we don't require data to be serializable since it's never transferred to other extension hosts, but that would have to change to make this work.

However, I realize that there's a difficulty for you in this scenario: the test items are requested in a "pull" model from VS Code. If it happens that someone is using your project explorer but never opened the Test Explorer or any documents which would cause items to be requested, it's impossible for you to provide a valid test item to runTests. I'll have to think about how to fix this. Suggestions welcome as well.

@connor4312 connor4312 added api and removed info-needed Issue requires more information from poster labels Jun 10, 2021
@connor4312 connor4312 added this to the June 2021 milestone Jun 10, 2021
@jdneo
Copy link
Member Author

jdneo commented Jun 10, 2021

Just an idea come up in my mind: make test request has an optional field that can associate with the test controller(s), like a id or something?

@connor4312
Copy link
Member

I think having an option for making test runs cancellable would work. The runs don't actually require "attached" test items, so you could just create a run manually and do what you need with it, and cancellation provides the necessary control for users.

@jdneo
Copy link
Member Author

jdneo commented Jun 11, 2021

I think having an option for making test runs cancellable would work. The runs don't actually require "attached" test items, so you could just create a run manually and do what you need with it, and cancellation provides the necessary control for users.

Sounds good. Does that mean the cancellation token will be provided when calling test.createTestRun?

@connor4312
Copy link
Member

There's now a cancellation token on the TestRun object.

@github-actions github-actions bot locked and limited conversation to collaborators Jul 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api insiders-released Patch has been released in VS Code Insiders testing Built-in testing support
Projects
None yet
Development

No branches or pull requests

3 participants
@connor4312 @jdneo and others