microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
163.04k stars 28.8k forks source link

[Test UI] Request: Options overload for CreateTestItem #127010

Closed JustinGrote closed 3 years ago

JustinGrote commented 3 years ago

Issue Type: Feature Request @connor4312

Since the controller createTestItem method is used a lot, there should be an options-style overload for easier usage and intellisense. Here's my implementation in my code today, if it were part of the interface the controller as the first parameter would not be necessary.

/**
 * Options for creating a managed test item
 *
 */
export interface CreateTestOptions {
    /** Uniquely identifies the test. Can be anything but must be unique to the controller */
    id: string
    /** A label for the testItem. This is how it will appear in the test explorer pane */
    label: string
    /** Which test item is the parent of this item. You can specify the test controller root here */
    parent: TestItem<any>
    /** A resource URI that matches the physical location of this test */
    uri?: Uri
    /** Custom data that never leaves this test */
    data?: any
}

/** Overload of {@link TestController.createTestItem} that accepts options */
export function createTestItem<TChild>(testController: TestController, options: CreateTestOptions) {
    testController.createTestItem<TChild>(
        options.id,
        options.label,
        options.parent,
        options.uri,
        options.data
    )
}

image

VS Code version: Code - Insiders 1.58.0-insider (9744231fc12f1aed21089180b3f0394d694bd2a2, 2021-06-23T05:13:08.071Z) OS version: Windows_NT x64 10.0.19043 Restricted Mode: No

System Info |Item|Value| |---|---| |CPUs|AMD Ryzen 7 4700U with Radeon Graphics (8 x 1996)| |GPU Status|2d_canvas: enabled
gpu_compositing: enabled
multiple_raster_threads: enabled_on
oop_rasterization: enabled
opengl: enabled_on
rasterization: enabled
skia_renderer: enabled_on
video_decode: enabled
vulkan: disabled_off
webgl: enabled
webgl2: enabled| |Load (avg)|undefined| |Memory (System)|15.36GB (4.68GB free)| |Process Argv|--crash-reporter-id 4453efaa-0821-4555-88e2-3de8bad9109e| |Screen Reader|no| |VM|0%|
Extensions (31) Extension|Author (truncated)|Version ---|---|--- emojisense|bie|0.8.0 markdown-checkbox|bie|0.1.3 vscode-opennewinstance|chr|0.0.7 bracket-pair-colorizer-2|Coe|0.2.1 esbuild-problem-matchers|con|0.0.2 gitlens|eam|11.5.1 filter-lines|ear|1.0.0 remotehub|Git|0.10.1 vscode-pull-request-github|Git|0.27.1 todo-tree|Gru|0.0.213 vscode-test-explorer|hbe|2.20.4 vscode-drawio|hed|1.5.0 change-case|hjd|1.0.2 git-graph|mhu|1.30.0 remote-containers|ms-|0.184.0 remote-ssh-nightly|ms-|2021.5.29700 powershell-preview|ms-|2021.6.1 test-adapter-converter|ms-|0.0.11 vscode-paste-image|mus|1.0.4 indent-rainbow|ode|7.5.0 docthis|oou|0.8.2 quicktype|qui|12.0.46 vscode-yaml|red|0.20.0 vscode-sort-json|ric|1.20.0 vscode-inline-values-powershell|Tyl|0.0.5 errorlens|use|3.2.7 vscode-icons|vsc|11.5.0 codetour|vsl|0.0.56 gistfs|vsl|0.2.9 gitmoji-vscode|Vtr|1.0.7 better-align|wwm|1.1.6
A/B Experiments ``` vsliv695:30137379 vsins829:30139715 vsliv368cf:30146710 vsreu685:30147344 python383:30185418 pythonvspyt602:30291494 vspor879:30202332 vspor708:30202333 vspor363:30204092 pythonvspyt639:30291487 pythontb:30258533 vspre833:30321513 pythonptprofiler:30281269 vshan820:30294714 pythondataviewer:30285072 vscus158:30321503 pythonvsuse255:30319630 vscorehov:30301224 vscod805:30301674 pythonvspyt200:30323110 vscextlang:30310088 vsccppwtct:30312693 ```
vscodebot[bot] commented 3 years ago

(Experimental duplicate detection) Thanks for submitting this issue. Please also check if it is already covered by an existing one, like:

jrieken commented 3 years ago

What the API usually does is to use dedicated arguments for mandatory field and an options bag for the rest, so it could be

createTestItem(id, label, parent, options?: { uri?, data? }): TestItem

The idea behind is to make mandatory properties more important and to give them more weight

JustinGrote commented 3 years ago

@jrieken the other point of the "everything" options bag in this case is that since it is an interface, it is easy to bring in JSON from an external tool with the same properties and cast it to the interface rather than having to unroll those properties for the constructor. I am a TypeScript novice so perhaps there is a better way if I have an external tool generating "test items" and outputting them to JSON?

jrieken commented 3 years ago

It sounds like a very specific case that you have. The options bag might be working best for you but we have to look at this from the API consistency point of view and ask yourself if this createXYZ-function is a good fit wrt to similar cases.

connor4312 commented 3 years ago

With https://github.com/microsoft/vscode/commit/6a76c62232208f54bc85f22572013b382e39cf9e we're down to only one optional property, so I don't think it's worth making a bag of them at this point.