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

Sample test runner for microsoft/vscode-extension-vscode#157 #186

Merged
merged 5 commits into from
May 24, 2019

Conversation

octref
Copy link
Contributor

@octref octref commented May 23, 2019

Benefits:

  • Test runner no longer coupled with Mocha 4 (options such as useColors are deprecated in Mocha 5)
  • Config file such as mocha.opts automatically resolved
  • Bugs such as Can't use mocha's dot reporter for extension tests vscode#56211 resolved since test runners should be run in child process. Mocha reporters uses process.stdout which in Extension Host context is not available. Jest / create-react-app uses spawn and AVA need child process as well
  • vscode-test doesn't need to depend on mocha. mocha should really be the extension's dev dependency.
  • Jest/AVA and other test framework can easily work this way
  • A lot more flexibility

I suggest that we standardize the run signature, much as we do for activate of extensions. Its current form has many problems:

function run(testsRoot: string, cb: (error: any, failures: number) => void): void;
  • testsRoot is absolute path to the test runner. Not very useful.
  • failures has hidden logic for determining success/fail. I suggest plain throw Error for failures.
  • 🚨 Unresolved Problem: Previously vscode is available through import in test files. Now since the tests run in another process vscode is not available...Is it possible to support this? If not I'll drop the child_process part, but that takes away all benefits above.

I suggest something like below:

function run(context: vscode.ExtensionTestContext): void;
async function run(context: vscode.ExtensionTestContext): void;

@bpasero What do you think?

@octref octref requested a review from bpasero May 23, 2019 01:00

// You can import and use all API from the 'vscode' module
// as well as import your extension to test it
// import * as vscode from 'vscode';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't use vscode here.

@octref
Copy link
Contributor Author

octref commented May 23, 2019

I can do something like this so everything works: cb83f39 (Following SO answer here: https://stackoverflow.com/a/32945536)

But if the test runner has to contain such workarounds, I would rather encapsulate it in vscode-test and export it, although it adds one more deps and limits the flexibility...

@bpasero
Copy link
Member

bpasero commented May 23, 2019

@octref can I ask why this is a PR for a sample? Is this an example for extensions how to implement a custom test runner?

@octref octref merged commit a2a2dce into master May 24, 2019
@octref octref added this to the May 2019 milestone May 24, 2019
@octref octref deleted the octref/test-runner branch July 3, 2019 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants