Skip to content

Proposal: add a contextValue to TestMessage #190277

@connor4312

Description

@connor4312

Motivation

There are sometimes actions users may want to take on test results. For example, as @ulugbekna references #189680 (comment), applying the "actual" value to test expectations is a common case. We do it using diagnostics and the proposed testObserver API in the selfhost test provider, but this is not a great experience. @orta's Jest extension adds a context menu to rerun the test and apply the snapshot.

Proposal

Simply enough, to add a contextValue on the TestMessage class (along with some extra menus where actions can be show)

export class TestMessage2 extends TestMessage {

	/**
	 * Context value of the test item. This can be used to contribute message-
	 * specific actions to the test peek view. The value set here can be found
	 * in the `testMessage` property of the following `menus` contribution points:
	 *
	 * - `testing/message/context` - context menu for the message in the results tree
	 * - `testing/message/content` - a prominent button overlaying editor content where
	 *    the message is displayed.
	 *
	 * For example:
	 *
	 * ```json
	 * "contributes": {
	 *   "menus": {
	 *     "testing/message/content": [
	 *       {
	 *         "command": "extension.deleteCommentThread",
	 *         "when": "testMessage == canApplyRichDiff"
	 *       }
	 *     ]
	 *   }
	 * }
	 * ```
	 *
	 * The command will be called with an object containing:
	 * - `test`: the {@link TestItem} the message is associated with, *if* it
	 *    is still present in the {@link TestController.items} collection.
	 * - `message`: the {@link TestMessage} instance.
	 */
	contextValue?: string;

	// ...
}

Limitations & Alternatives

The main limitation arises if the information on the TestMessage and TestItem is insufficient to accomplish the necessary tasks. For snapshot testing, it should be sufficient to apply the actualOutput, but if it's not, then the runner is stuck re-running the test.

A fix for this would be if we ensure that the TestMessage instance passed to the executed command is the same as the one initially created in the test result, allow the consumer to use a WeakMap approach. However, this requires retaining test messages in the extension host, which we don't currently do and may be burdensome.

one clever workaround for this would be having the TestMessage instance be a "shell" class that gets/sets properties on an inner class. We could wipe out this inner instance when we're done with the result in the extension host, and re-hydrate it when the command is executed. Then we would only have the base memory usage from the shell class and avoid holding potentially huge results indefinitely.

An alternative would be to allow the TestMessage to have a list of Commands. This is more flexible, namely in that custom arguments can be supplied to the command, but we'd have to re-invent how to determine where actions are shown in the UI.

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions