[vscode] Support TestMessage#contextValue #13176

rschnekenbu · 2023-12-14T08:53:56Z

What it does

Provides support for 1.84 vscode API TestMessage#contextValue optional property. This also adds one introduced menu using this API (testing/message/context). See https://code.visualstudio.com/updates/v1_84#_finalized-testmessagecontextvalue-api
The other menu is not relevant currently, as the test API implementation is not as advanced as the vscode one (no diff editor with actual and expected compared for example).

Fixes #13144

Contributed on behalf of STMicroelectronics

How to test

The test can be done installing the following extension:

zip:
test-extension-sample-0.0.1.zip
src:
test-extension-sample-0.0.1-src.zip

This adds a test controller that required the creation of a test run profile before being used (Command palette > Create Test Run Profile). Once the test has run, the results are shown. On the test results, there is a new context menu action that displays a message information with the arguments passed to this command. The expected arguments are described in the comment of TestMessage#contextValue (see theia.d.ts - line to be added once commited)

Note that since the testing/message/content extension menu is not yet implemented, the extension will show a warning at startup.

Follow-ups

Support of the testing/message/content extension menu. This is tracked under #13145

Review checklist

As an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

As a reviewer, I agree to behave in accordance with the review guidelines

rschnekenbu · 2023-12-14T13:30:13Z

@tsmaeder, as you worked on the test API, could you have a look to this one?

tsmaeder

Just two remarks.

tsmaeder · 2023-12-18T15:51:21Z

packages/plugin-ext/src/common/test-types.ts

@@ -131,3 +132,23 @@ export namespace TestItemReference {
    }
 }

+export interface TestMessageArg {
+    typeTag: '$type_test_message_arg',


Do we really need this or can we just send an array of [TestItemReference, TestMessageDTO]?

That was easier to detect the kind of message, as we send an array of 2 elements with one optional and the other with almost everything optional except 1 property. But this is definitely not mandatory.
That can be removed and we can have the following code for TestMessageArg.is():

Suggested change

typeTag: '$type_test_message_arg',

export interface TestMessageArg {

testItemReference: TestItemReference | undefined,

testMessage: TestMessageDTO

}

export namespace TestMessageArg {

export function is(arg: unknown): arg is TestMessageArg {

return isObject<TestMessageArg>(arg)

&& isObject<TestMessageDTO>(arg.testMessage)

&& typeof arg.testMessage.message === 'string';

}

I was more thinking of making the argument transformer method recursive: we are already detecting if it's an array, but the assumption is that the contents can only be TestItemReferences

tsmaeder · 2023-12-18T16:22:46Z

packages/test/src/browser/view/test-run-widget.tsx

@@ -216,6 +218,12 @@ export class TestRunTreeWidget extends TreeWidget {
                    onDidAddTestOutput: Event.map(node.parent.run.onDidChangeTestOutput, evt => evt.filter(item => item[0] === node.item).map(item => item[1]))
                };
                this.uiModel.selectedTestState = node.parent.run.getTestState(node.item);
+                if (this.uiModel.selectedTestState && TestFailure.is(this.uiModel.selectedTestState.state)) {


Shouldn't this code live in the uiModel as it's the owner of the selectedTestState?

Thanks for noticing! Indeed, that should happen in the uiModel. I will update the code and move this part of the code on the selectedTestState mutator.

tsmaeder · 2023-12-19T09:30:41Z

When testing, I never get anything in the info popup for the message context except: Message Context with args: [object Object]. In instance, I got a stack overflow:

4rpc-message-encoder.ts:151 Uncaught (in promise) Error: Error during encoding: 'Maximum call stack size exceeded'
    at MsgPackMessageEncoder.encode (rpc-message-encoder.ts:151:23)
    at MsgPackMessageEncoder.request (rpc-message-encoder.ts:137:14)
    at RpcProtocol.sendRequest (rpc-protocol.ts:161:22)
    at proxy-handler.ts:74:45

This happened when doing this:

left-click "sample-child 1" in the Test Explorer view
right-click "sample-child 1" in the Test Run view and select "Message Context" from the context menu

Not sure whether that is due to the test plugin or the PR code.

packages/plugin-ext/src/plugin/tests.ts

packages/plugin-ext/src/main/browser/menus/plugin-menu-command-adapter.ts

rschnekenbu · 2023-12-19T10:59:15Z

When testing, I never get anything in the info popup for the message context except: Message Context with args: [object Object]. In instance, I got a stack overflow:
4rpc-message-encoder.ts:151 Uncaught (in promise) Error: Error during encoding: 'Maximum call stack size exceeded'
    at MsgPackMessageEncoder.encode (rpc-message-encoder.ts:151:23)
    at MsgPackMessageEncoder.request (rpc-message-encoder.ts:137:14)
    at RpcProtocol.sendRequest (rpc-protocol.ts:161:22)
    at proxy-handler.ts:74:45
This happened when doing this:

left-click "sample-child 1" in the Test Explorer view

right-click "sample-child 1" in the Test Run view and select "Message Context" from the context menu

Not sure whether that is due to the test plugin or the PR code.

I tried again, using the extension provided in the PR and with latest version, I do not get these stack error messages on sample-child1. I tested on both browser and electron version. I see the difference with your message information text, I will push a new version of the extension with pretty printed objects.

I can see however a call stack error while right clicking on the Test Run root object. I will fix that.

rschnekenbu · 2023-12-19T11:43:53Z

I updated the PR with more checks on arguments processing. Element tree in Test Run widget can also be TestRun, which was not caught in argument processor and forwarded as such without cleaning.

Here is the version of the test extension with stringified argument:
zip:
test-extension-sample-0.0.3.zip
src:
test-extension-sample-0.0.3-src.zip

tsmaeder · 2023-12-20T08:54:56Z

With the latest plugin, I always get "Message Context with args: undefined" when I click the context menu. What gives?

rschnekenbu · 2023-12-20T09:40:23Z

With the latest plugin, I always get "Message Context with args: undefined" when I click the context menu. What gives?

Sorry for that, @tsmaeder! I am pretty sure I had check thoroughly the changes :/ In certain cases, TestMessage.is() was returning false despite the element passed being one. I updated the PR so this should not happen anymore.
In the case the test items are not in failure or the selected element in the tree is not a TestItem, the command shall return the kind of message that you have, as the expected arguments cannot be computed. In fact, TestMessage is only available in case of test failures. I assume there it is up to the extension to check the enablement of the command.
I will squash and resolve conflicts once the PR is approved if it is OK for you.

tsmaeder

I think your original code was correct as far as the typing goes: the "location" field in TestMessage should probably be optional. But that's another problem. Lgtm now.

rschnekenbu · 2023-12-20T10:48:10Z

Thanks a lot, @tsmaeder! I will rebase and resolve conflicts then.

Also adds the menu mapping for testing/message/context vscode menu extension. contributed on behalf of STMicroelectronics Signed-off-by: Remi Schnekenburger <[email protected]> review comments review comments - avoid passing TestRun in the plugin menu command adapter review comments TestMessage location can be undefined, so the TestMessage.is() was returning false despite being one.

JonasHelming · 2023-12-20T14:09:21Z

@tsmaeder Is this approved?

rschnekenbu force-pushed the issues/13144 branch from 854f0e0 to d975a01 Compare December 14, 2023 10:55

JonasHelming requested a review from tsmaeder December 14, 2023 13:31

tsmaeder reviewed Dec 18, 2023

View reviewed changes

rschnekenbu requested a review from tsmaeder December 19, 2023 09:25

tsmaeder requested changes Dec 19, 2023

View reviewed changes

packages/plugin-ext/src/plugin/tests.ts Show resolved Hide resolved

packages/plugin-ext/src/main/browser/menus/plugin-menu-command-adapter.ts Show resolved Hide resolved

rschnekenbu mentioned this pull request Dec 19, 2023

[vscode] new contextual menu for TestMessages: testing/message/context and testing/message/content #13145

Closed

tsmaeder approved these changes Dec 20, 2023

View reviewed changes

rschnekenbu force-pushed the issues/13144 branch from 91fb671 to 17b9b39 Compare December 20, 2023 10:56

rschnekenbu requested a review from tsmaeder December 20, 2023 11:22

JonasHelming merged commit 7dcce3e into eclipse-theia:master Dec 20, 2023
14 checks passed

vince-fugnitto added this to the 1.45.0 milestone Dec 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[vscode] Support TestMessage#contextValue #13176

[vscode] Support TestMessage#contextValue #13176

rschnekenbu commented Dec 14, 2023

rschnekenbu commented Dec 14, 2023

tsmaeder left a comment

tsmaeder Dec 18, 2023

rschnekenbu Dec 19, 2023

tsmaeder Dec 19, 2023

tsmaeder Dec 18, 2023

rschnekenbu Dec 19, 2023

tsmaeder commented Dec 19, 2023

rschnekenbu commented Dec 19, 2023

rschnekenbu commented Dec 19, 2023

tsmaeder commented Dec 20, 2023

rschnekenbu commented Dec 20, 2023 •

edited

Loading

tsmaeder left a comment

rschnekenbu commented Dec 20, 2023

JonasHelming commented Dec 20, 2023

-    typeTag: '$type_test_message_arg',
+export interface TestMessageArg {
+    testItemReference: TestItemReference | undefined,
+    testMessage: TestMessageDTO
+}
+export namespace TestMessageArg {
+    export function is(arg: unknown): arg is TestMessageArg {
+        return isObject<TestMessageArg>(arg)
+            && isObject<TestMessageDTO>(arg.testMessage)
+            && typeof arg.testMessage.message === 'string';
+    }

[vscode] Support TestMessage#contextValue #13176

[vscode] Support TestMessage#contextValue #13176

Conversation

rschnekenbu commented Dec 14, 2023

What it does

How to test

Follow-ups

Review checklist

Reminder for reviewers

rschnekenbu commented Dec 14, 2023

tsmaeder left a comment

Choose a reason for hiding this comment

tsmaeder Dec 18, 2023

Choose a reason for hiding this comment

rschnekenbu Dec 19, 2023

Choose a reason for hiding this comment

tsmaeder Dec 19, 2023

Choose a reason for hiding this comment

tsmaeder Dec 18, 2023

Choose a reason for hiding this comment

rschnekenbu Dec 19, 2023

Choose a reason for hiding this comment

tsmaeder commented Dec 19, 2023

rschnekenbu commented Dec 19, 2023

rschnekenbu commented Dec 19, 2023

tsmaeder commented Dec 20, 2023

rschnekenbu commented Dec 20, 2023 • edited Loading

tsmaeder left a comment

Choose a reason for hiding this comment

rschnekenbu commented Dec 20, 2023

JonasHelming commented Dec 20, 2023

rschnekenbu commented Dec 20, 2023 •

edited

Loading