Implementation of testing api for testing vscode extensions language features

[JPinkney]

What does this PR do?

This PR introduces testing api that allows you to talk to a specific vscode extensions language features. It gives the ability to create tests that call into a vscode language extension for two reasons:

1. Languages team can run tests against newer version of language plugins so we know if a language plugin can be updated or something needs to be fixed
2. Che tests can talk directly with the language server so we know that if the language server request fails then it's an actual failure, rather than a test failing for a different reason and making it look like it was the cause

Examples of such tests can be found in: https://github.com/jpinkney/che-java-tests though they're WIP.

What issues does this PR fix or reference?

eclipse-che/che#14409

Release Notes

Docs PR

[tsmaeder]

One thing that is not clear to me is how tests are loaded and run.

[JPinkney]

I originally experimented with having the test loader as a che theia plugin but over time it felt like it made more sense to go with the strategy where each plugin will load the tests themselves. This allows the tests to have a certain level of configurability, such as how long the tests should try to run before timing out, and gives the additional benefit of not having to lock into using specific testing frameworks.

I've done some documentation on how the tests will work in https://github.com/jpinkney/che-java-tests#running-these-tests-automatically-when-a-workspace-starts but the basic idea is that you first start a workspace using a devfile that references a theia testing plugin (like https://github.com/jpinkney/che-java-tests). When the workspace starts the testing plugin loads in all the tests and communicates with the testing-api that is available in this PR

[JPinkney]

@benoitf @tsmaeder Do you mind taking a look again ?

[benoitf]

Hello
will look more deeply

I think there are some missings compilation.tsconfig.json for new folders ?

I was wondering why we need another root namespace for the test-service ? '@eclipse-che/testing-service and not included it in @eclipse-che/plugin under a test namespace

[JPinkney]

@benoitf I can move it to under the @eclipse-che/plugin in a test namespace. Should I also move everything in testing-service-ext to under plugin-ext?

[azatsarynnyy]

I'm +1 for keeping everything under a single che namespace.

[tsmaeder]

I was wondering why we need another root namespace for the test-service ? '@eclipse-che/testing-service and not included it in @eclipse-che/plugin under a test namespace

I think it makes perfect sense to have a separate API namespace: the che namespace essentially deals with communicating with the WSMaster API: workspace, devfiles, etc. The testing namespace has no substantial connection with that topic.
Secondly, by keeping it separate, the testing API could moved upstream eventually. Marrying it to Che workspaces would make that impossbile.
Thirdly, it could make sense not to ship the testing API in a production version of Che, but only in a development version. Think of build.sh --production.
So I'm all for keeping it separate.

[benoitf]

sorry Thomas, but for all the reasons you're referring I'm even more for included in che namespace

che namespace is what you have when using che instead of theia, its not for only using workspace API. (it's just that it's evolving on the needs)
If people write plugin's for che, they don't want to know if a separate API will be provided or not.
And if they need to find yet another namespace when writing code, it's even more complex.

Secondly, by keeping it separate, the testing API could moved upstream eventually. Marrying it to Che workspaces would make that impossbile.

I don't see where it would make it impossible, you can link namespace. And what if people already starts to use @eclipse-che/testing-service it would break for them as well.

Also I'm talking about che namespace. Che namespace is what you get when using Theia in Che, it's not only che workspace

Thirdly, it could make sense not to ship the testing API in a production version of Che, but only in a development version. Think of build.sh --production.

Why you would cut that ? If I use VS Code I can run tests on the 'production binary'. As a user I would want to test my plug-in on che.openshift.io for example.
Also people will never know if they could deploy or not the plugin in that case.

[tsmaeder]

@benoitf the testing API is not che-specific, there is really no reason to include it in the Che namespace other than pure convenience. If Che is mentioned elsewhere in the PR, we should probably change those occurrences. We even thought about doing all the works upstream, but decided on doing it downstream because it's simpler to proceed with the task.

[tsmaeder]

you can link namespace

Could you explain what you mean by this? I don't understand.

[tsmaeder]

As always, I have t couple of nitpicks, but otherwise I like this PR.

[che-bot]

Can one of the admins verify this patch?

[tsmaeder]

lgtm!

[benoitf]

please merge test service into @eclipse-che/plugin into test namespace or language.test

[benoitf]

All these methods should be well documented, this is what the user will see when using code completion on the namespace

[benoitf]

Hi, are you following some rules to have sometimes Promise vs PromiseLike ?

[tsmaeder]

I this case, it should reflect what ever the "non-testing" version of the function uses.

[benoitf]

ok b/c I see in "non-testing"

provideRenameEdits(document: TextDocument, position: Position, newName: string, token: CancellationToken): ProviderResult<WorkspaceEdit>

provideDocumentSymbols(document: TextDocument, token: CancellationToken): ProviderResult<SymbolInformation[] | DocumentSymbol[]>;

and renameEdits is using PromiseLike while documentSymbols is using Promise

[JPinkney]

Types are different because when I was creating the d.ts file I used the types that came out of https://github.com/eclipse-theia/theia/blob/master/packages/plugin-ext/src/common/plugin-api-rpc.ts#L1146 not the types that came out of the theia.d.ts

[benoitf]

yes I see that but we should map to external/public stuff, not implementation, no ?

[benoitf]

Why do we have WorkspaceEditDto and not WorkspaceEdit ?

Lots of types should come from @theia/plugin no ?

[JPinkney]

https://github.com/eclipse-theia/theia/blob/master/packages/plugin-ext/src/plugin/languages.ts#L543 it's just following what provideRenameEdits uses

[benoitf]

IMHO it should follow the declaration, not the implementation

[JPinkney]

It's following the LanguageExt interface: https://github.com/eclipse-theia/theia/blob/master/packages/plugin-ext/src/common/plugin-api-rpc.ts#L1196

[benoitf]

where is used that import ?

[tsmaeder]

please merge test service into @eclipse-che/plugin into test namespace or language.test

@JPinkney and I discussed this in yesterdays daily and we both agree that the testing stuff should be a separate module and API namespace if that's what you object to (for the reasons outlined before).

[benoitf]

why are we not using https://github.com/eclipse-theia/theia/blob/master/packages/plugin/src/theia.d.ts#L5448 (I would say all types should come from @theia/plugin no) ?

[JPinkney]

@benoitf I've tried switching the types to be compatible with @theia/plugin but I'm not entirely sure if its possible

in che-languages-test-api when we call:

(languagesExt as LanguagesExt).$provideCompletionItems(......)

we are recieving back

Promise<CompletionResultDto | undefined>

from https://github.com/eclipse-theia/theia/blob/master/packages/plugin-ext/src/plugin/languages/completion.ts#L39. We aren't actually calling the delegate that has the type theia.CompletionItemProvider https://github.com/eclipse-theia/theia/blob/master/packages/plugin-ext/src/plugin/languages/completion.ts#L34.

Instead, we are getting back the converted result that happens after the delegate is called.

When I tried to modify che-languages-test-api to support @thiea/plugin types like

$provideImplementation(pluginID: string, document: TextDocument, position: Position, token: CancellationToken): ProviderResult<Definition | DefinitionLink[]> {
        return this.pluginHandleRegistry.lookupLanguagesExtForPluginAndAction(pluginID, 'implementation').then(({ languagesExt, handle }) => {
            return (languagesExt as LanguagesExt).$provideImplementation(handle, document.uri, { column: position.character, lineNumber: position.line }, token);
        });
    }

it leads to a lot of incompatible types for all of the methods (provideCompletionItems, provideImplementation, etc etc) in that class.

It looks like this is because the LanguagesExt interface is using the types from plugin-api-rpc-model' and che-languages-test-api is trying to use @theia/plugin types

I do have a working branch that has moved the testing api into @eclipse-che/api though.

[tsmaeder]

I do have a working branch that has moved the testing api into @eclipse-che/api though.

Let's please not do that. It's wrong! Before we do that, let's discuss it in a call.

[ericwill]

Superseded by #802