Test explorer sort order is incorrect

[garyhuntddn]

Type: Bug

See #163449 which caused this regression.

Tests that don't provided a "sortText" are sorted based upon their code location.

TestCase()'s in NUnit tests in C# don't provide that sortText property (it is always null) and this results in the test cases not being sorted as they refer to the same method (and therefore the same code location).

A simple C# example:

[TestCase( "A" )]
[TestCase( "B" )]
[TestCase( "C" )]
public void MyTest( String arg ) {}

Will result in the Test Explorer showing this test MyTest, with it's children A, B, C in a seemingly random order.

The code causing this is around lines 1226-1240 of /src/vs/workbench/contrib/testing/browser/testingExplorerView.ts.

I can confirm through a local build of Code-OSS that changing lines 1236 and 1237 to:

const sa = a.test.item.sortText || a.test.item.label;
const sb = b.test.item.sortText || b.test.item.label; 

Fixes the issue for me - but may in itself cause #163449 to reoccur so a discussion is required.

I'm happy to develop and propose a PR - including an option I've toyed with where we add an option of sort by "label" in addition to "location" to maintain backward compatibility.

VS Code version: Code 1.92.0 (Universal) (b1c0a14de1414fcdaa400695b4db1c0799bc3124, 2024-07-31T23:26:45.634Z) OS version: Darwin arm64 23.5.0 Modes:

System Info

|Item|Value| |---|---| |CPUs|Apple M2 (8 x 2400)| |GPU Status|2d_canvas: enabled
canvas_oop_rasterization: enabled_on
direct_rendering_display_compositor: disabled_off_ok
gpu_compositing: enabled
multiple_raster_threads: enabled_on
opengl: enabled_on
rasterization: enabled
raw_draw: disabled_off_ok
skia_graphite: disabled_off
video_decode: enabled
video_encode: enabled
webgl: enabled
webgl2: enabled
webgpu: enabled
webnn: disabled_off| |Load (avg)|2, 4, 5| |Memory (System)|16.00GB (0.08GB free)| |Process Argv|. --crash-reporter-id ae3c96fd-91f6-44f5-8f8b-e98a6f753843| |Screen Reader|no| |VM|0%|

Extensions (50)

Extension|Author (truncated)|Version ---|---|--- atlascode|atl|3.0.10 vscode-tailwindcss|bra|0.12.5 emotion-auto-css|ccj|0.0.4 npm-intellisense|chr|1.4.5 vscode-markdownlint|Dav|0.55.0 vscode-eslint|dba|3.0.10 make-hidden|dev|4.0.4 githistory|don|0.6.20 xml|Dot|2.5.1 gitlens|eam|15.2.3 EditorConfig|Edi|0.16.4 prettier-vscode|esb|10.4.0 vscode-firefox-debug|fir|2.9.10 dotnet|for|0.0.4 dotnet-test-explorer|for|0.7.8 vscode-pull-request-github|Git|0.92.0 vscode-graphql|Gra|0.11.2 vscode-graphql-syntax|Gra|1.3.6 todo-tree|Gru|0.0.226 vscode-antlr4|mik|2.4.6 vscode-docker|ms-|1.29.1 csdevkit|ms-|1.8.14 csharp|ms-|2.39.29 vscode-dotnet-runtime|ms-|2.1.1 vscodeintellicode-csharp|ms-|2.1.11 vscode-edge-devtools|ms-|2.1.5 data-workspace-vscode|ms-|0.5.0 mssql|ms-|1.23.0 sql-bindings-vscode|ms-|0.4.0 sql-database-projects-vscode|ms-|1.4.3 remote-containers|ms-|0.380.0 remote-ssh|ms-|0.112.0 remote-ssh-edit|ms-|0.86.0 extension-test-runner|ms-|0.0.11 hexeditor|ms-|1.10.0 js-debug-nightly|ms-|2024.8.217 powershell|ms-|2024.2.2 remote-explorer|ms-|0.4.3 remote-server|ms-|1.5.2 vs-keybindings|ms-|0.2.1 vscode-github-issue-notebooks|ms-|0.0.130 vscode-selfhost-test-provider|ms-|0.3.25 vscode-jest|Ort|6.2.5 vscode-yaml|red|1.15.0 code-spell-checker|str|3.0.1 code-spell-checker-british-english|str|1.4.10 vscode-styled-components|sty|1.7.8 vscode-mdx|uni|1.8.9 errorlens|use|3.20.0 vscode-mdx-preview|xyc|0.3.3

A/B Experiments

``` vsliv368:30146709 vspor879:30202332 vspor708:30202333 vspor363:30204092 vscoreces:30445986 vscod805cf:30301675 binariesv615:30325510 vsaa593cf:30376535 py29gd2263:31024239 c4g48928:30535728 azure-dev_surveyone:30548225 962ge761:30959799 pythongtdpath:30769146 welcomedialogc:30910334 pythonnoceb:30805159 asynctok:30898717 pythonregdiag2:30936856 pythonmypyd1:30879173 2e7ec940:31000449 pythontbext0:30879054 accentitlementsc:30995553 dsvsc016:30899300 dsvsc017:30899301 dsvsc018:30899302 cppperfnew:31000557 dsvsc020:30976470 pythonait:31006305 dsvsc021:30996838 bdiig495:31013172 pythoncenvpt:31062603 a69g1124:31058053 dvdeprecation:31068756 dwnewjupyter:31046869 impr_priority:31102340 refactort:31108082 ccplti:31103428 pythonrstrctxtcf:31103194 wkspc-onlycs-t:31106322 ```

[garyhuntddn]

There is possibly another solution that I haven't investigated - whereby the C# test resolver populates the aforementioned sortText property.

[garyhuntddn]

Also note that the context menu uses a different sort order to the one introduced to fix #163449.

/src/vs/workbench/contrib/testing/browser/testingDecorations.ts

        testItems.sort((a, b) => {
            const ai = a.testItem.test.item;
            const bi = b.testItem.test.item;
            return (ai.sortText || ai.label).localeCompare(bi.sortText || bi.label);
        });

Follows my gut reaction fix to use the label if the sortText isn't provided - so this should have been part of the original fix but the python users who originally had the issue have ignored that aspect and only provided a change to the sort order in the explorer.

[garyhuntddn]

@connor4312 would you be interested in me putting together a PR for this one? any preference on the fix? a new sort order option based purely on name? reverting back the original fix for python?

[connor4312]

TestCase()'s in NUnit tests in C# don't provide that sortText property (it is always null) and this results in the test cases not being sorted as they refer to the same method (and therefore the same code location).

The tests should be sorted in the order that they're provided by the extension, if they're at the same location and in the absence of explicit sortText. If that's not happening then that's a bug, and I'd welcome a PR that fixes that 🙂

[garyhuntddn]

Thanks for the reply Connor - I'll put together a PR and some fresh tests/examples

On Wed, 21 Aug 2024 at 20:28, Connor Peet @.***> wrote:

TestCase()'s in NUnit tests in C# don't provide that sortText property (it is always null) and this results in the test cases not being sorted as they refer to the same method (and therefore the same code location).

The tests should be sorted in the order that they're provided by the extension, if they're at the same location and in the absence of explicit sortText. If that's not happening then that's a bug, and I'd welcome a PR that fixes that 🙂

— Reply to this email directly, view it on GitHub https://github.com/microsoft/vscode/issues/224679#issuecomment-2302860895, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKT545XDCJH2Z66ED75YITZSTS6TAVCNFSM6AAAAABL5Z66XKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMBSHA3DAOBZGU . You are receiving this because you authored the thread.Message ID: @.***>

--

Gary Hunt

*Senior Developer, Data Delivery Network Ltd*Mob: 07970 908428

DISCLAIMER :This Email contains confidential information some or all of which may be legally privileged. It is intended for the recipient only. If an addressing or transmission error has misdirected this Email, please notify the above author.