Proposal: add a contextValue to TestMessage

[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.

[connectdotz]

@connor4312, a bit of late feedback regarding the snapshots: our use case will fall into the "re-run test" category. It would be unlikely for us to update snapshots directly based on the test output for the following reasons:

1. Avoid reinventing the wheel. Jest already knows how to update snapshots; we should not replicate the same logic, which is not as trivial as it might seem.
2. Avoid snapshot version compatibility issues. Even if we were to decide to reinvent the wheel, the concerns of accuracy, version maintenance, and synchronization with Jest present a nightmare scenario we would like to avoid if possible.

For us, updating snapshots is really just a test run with a special flag. Therefore, it seems natural for the action to sit with the rest of the run/debug menu items. The TestMessage contextMenu will not add any additional benefit to our use case.

Just to clarify, we don't object to this feature, especially when it helps other extensions and use cases. This is just some context for reference.

[connor4312]

Sounds good. However, on "Jest already knows how to update snapshots; we should not replicate the same logic" -- you could use this action to trigger Jest's snapshot update, you don't necessarily need to do the update by hand.