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

Testing Extension API: Enrich TestMessage #133028

Open
hediet opened this issue Sep 13, 2021 · 1 comment
Open

Testing Extension API: Enrich TestMessage #133028

hediet opened this issue Sep 13, 2021 · 1 comment
Assignees
Labels
testing Built-in testing support under-discussion Issue is under discussion for relevance, priority, approach

Comments

@hediet
Copy link
Member

hediet commented Sep 13, 2021

Motivation

I think it would be very useful if VS Code (or an extension) could help fixing failing tests, in particular those where a deep strict equal assertion failed, like this:

recording

Problem

However, there is no nice way extensions can do that if the test provider does not expose the exact shape of the actual data. Currently there is not even a way test providers can expose data at all.

This is the current definition of a TestMessage:

export class TestMessage {
    /**
        * Human-readable message text to display.
        */
    message: string | MarkdownString;

    /**
        * Expected test output. If given with {@link actualOutput}, a diff view will be shown.
        */
    expectedOutput?: string;

    /**
        * Actual test output. If given with {@link expectedOutput}, a diff view will be shown.
        */
    actualOutput?: string;

    /**
        * Associated file location.
        */
    location?: Location;

    /**
        * Creates a new TestMessage that will present as a diff in the editor.
        * @param message Message to display to the user.
        * @param expected Expected output.
        * @param actual Actual output.
        */
    static diff(message: string | MarkdownString, expected: string, actual: string): TestMessage;

    /**
        * Creates a new TestMessage instance.
        * @param message The message to show to the user.
        */
    constructor(message: string | MarkdownString);
}

expectedOutput and actualOutput are shown to the user and thus are optimized for being human-readable and not machine-readable.

When implementing the motivating feature, this causes a conflict: Either actualOutput is nice to read by humans or it truthfully reflects the actual value that can be used to as expected output to fix the test.

Proposed Solution 1

Allow test providers to attach arbitrary metadata to test messages.

export class TestMessage {
    metadata?: Record<string, JSONValue>;
    // ...
}

Consumers of such test message can set/read that metadata:

const message = new vscode.TestMessage(tryMakeMarkdown(err));
message.location = location ?? testFirstLine;
// TODO: Allow to attach meta-data to the message!
// Keep actual/expected as human readable text and add
// JSON encoded metadata, readable by the code-action that can fix failing deep strict equal assertions.
message.actualOutput = prettyPrint(actualOutput);
message.expectedOutput = prettyPrint(expectedOutput);
message.metadata = {
  kind: 'assertStrictDeepEqualFailed',
  actualOutputJSON: toJSON(actualOutput)
};
task.failed(tcase!, message, duration);

The downside of this approach is that metadata has no semantics and is not standardized and extensions can do anything they like.

Proposed Solution 2: Introduce TestMessage.actualOutputJSON

However, this might only apply to JavaScript based test-runners.

Proposed Solution 3: Support extension-private meta data

Like solution 1, but this metadata is only visibile to the extension that produced the test item.
This extension has to export APIs so that other extension can query this metadata.

@connor4312 connor4312 added the testing Built-in testing support label Sep 13, 2021
@connor4312
Copy link
Member

As I mentioned in PM, the usual solution to this problem in our API is to preserve instance identity -- allowing for Map/WeakMap usages -- and then pass the instance back in actions. I think in this case we can do that, and then provide message-related contribution points (e.g. inline actions in the test run 'history' view, maybe context menu actions on the line) which would pass the message instance back up to the extension.

The kind of difficult thing is this requires that we keep the original TestMessage instances around in the extension host, and in the test history view we show lots of old messages. Also, if the window is reloaded, then associated instances for older runs will of course be lost.

@mattbierner do you have thoughts on the most idiomatic way to approach this?

@connor4312 connor4312 added the under-discussion Issue is under discussion for relevance, priority, approach label Sep 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing Built-in testing support under-discussion Issue is under discussion for relevance, priority, approach
Projects
None yet
Development

No branches or pull requests

2 participants