vshaxe / haxe-test-adapter

Haxe Test Explorer for Visual Studio Code
MIT License
29 stars 5 forks source link

Pending buddy tests should not be marked as passing in VS Code #26

Closed theJenix closed 1 year ago

theJenix commented 1 year ago

Unclear if this is a logic bug per se but I think it is definitely a DX issue; when using haxe-test-adapter along with buddy BDD, pending tests are displayed in VS Code's Testing UI as "Passed".

Pending tests are tests with no test body or behaviors; they are declarations that have yet to be implemented. In Buddy output, they are displayed differently from passed tests, so I would expect them to be similarly differentiated in VS Code's UI.

Would you consider marking pending tests as skipped? And/or, adding (PENDING) to the start of a pending test? I'm happy to submit a PR if either or both of these ideas (or something else) sound good.

AlexHaxe commented 1 year ago

so you are suggesting to change https://github.com/vshaxe/haxe-test-adapter/blob/926318dded5c6786a74cc3a59966857a9e15dfee/test-adapter/_testadapter/buddy/Reporter.hx#L100 from Success to Ignore?

theJenix commented 1 year ago

That would do it, but I don't know the implications to the rest of the test adapter of marking the test as ignore. So either that, or adding a Skipped TestState, using it on that line, and adding that to the switch statement in HaxeTestController

AlexHaxe commented 1 year ago

vscode only knows passed, failed, errored and skipped states, so we have to stick with those.

Ignore is currently used with multiple test frameworks to report testcases that weren't run (usually there is some sort of @Ignore metadata involved). when reporting results to vscode test api Ignore turns to skipped state.

so an empty / pending buddy test would show a skipped symbol with Ignore, and since there are no tests / no implementation, technically that testcase didn't "run" / didn't contribute anything to cover any code, so skip should be fine.

theJenix commented 1 year ago

Awesome; I can make the change and submit a PR. How do you feel about also adding (PENDING) to the test name? Buddy does this in it's console reporter and its a good combo (the color change + the text) to indicate tests that still need to be written.

AlexHaxe commented 1 year ago

adding (PENDING) can work. I'm not using buddy myself, but I assume that test names are usually rather long because of their descriptive nature, so adding an extra suffix/prefix will make them even longer. and we already have a "skipped" icon in front of tests that were skipped or are pending. on the other hand there seems to be no filter option to only show skipped tests (or I haven't found it), so having a common search term like (PENDING) will help in finding testcases that still need implementing.

so yeah, make that PR and play around with different options for naming to see what works best in real world scenarios.

theJenix commented 1 year ago

Thanks for taking care of this, and sorry I let it drop; day job got in the way. This will be a huge improvement for folks like me using Buddy with VSCode Testing and this adapter!

AlexHaxe commented 1 year ago

no worries. to use it all you need to do is haxelib install test-adatper, you don't have to compile Haxe test adapter extension (or wait for a marketplace release), since that part hasn't changed.