test: migrate to xUnit

[alsi-lawr]

Migrates the existing test suite from MSTest/VSTest to xUnit v3, and Microsoft Testing Platform.

Separates unit and Neovim process/RPC integration tests into distinct projects and CI steps without adding new test coverage.

Blocked by

• refactor: simplify remote plugins #37

Closes #29

[alsi-lawr]

Switched back to draft to do a little cleanup from the remote plugin removal PR

[justinmk]

why is Shouldly needed? avoid deps if possible

[alsi-lawr]

why is Shouldly needed? avoid deps if possible

It's not needed, but it's a very nice FOSS assertion framework that makes the tests easier to read. Can remove if desired, but it's pretty standard/normal for dotnet libraries to use shouldly/fluentassertions, granted it does defeat the project's stated goal of having a minimal maintenance burden

[justinmk]

that makes the tests easier to read.

as i mentioned, this (and any api client) should be simple enough for that not to matter.

also the BDD test description format is just stupid (i wish we didn't have it in neovim core). although it looks like Shouldly is trying to be like "jest", which is also just sugar without a real purpose, and doesn't justify an extra dep

[alsi-lawr]

that makes the tests easier to read.

as i mentioned, this (and any api client) should be simple enough for that not to matter.

also the BDD test description format is just stupid (i wish we didn't have it in neovim core). although it looks like Shouldly is trying to be like "jest", which is also just sugar without a real purpose, and doesn't justify an extra dep

I'm removing shouldly now, but what's the alternative test description format you want to adopt? I don't personally think the existing TestMessageDeserialization is better, every test starts with Test*, which is redundant since it's in a test project. If it were f# I'd just write actual english scenario names like

let ``A request to embedded Neovim deserializes its response`` () =
...

[justinmk]

I don't personally think the existing TestMessageDeserialization is better, every test starts with Test*, which is redundant since it's in a test project.

that's completely fine, and again, this repo isn't going to have zillions of tests so it doesn't matter.

If it were f# I'd just write actual english scenario names like

that's BDD format. it is awkward to reference in discussions, code comments, etc, because it's just a free-form name, the test doesn't have an "identifier".

it also seems to encourage no restraint whatsoever when people decide test names.

[justinmk]

remind me why we need separate unit vs integ tests? all tests should run actualy Nvim instances, it's extremely fast. so there is no need for "fast" (unit) vs "slow" (integ).

every single test in Neovim core, out of 9000+ tests, runs a new instance of nvim

[alsi-lawr]

The unit/integration distinction is not intended to be fast versus slow. It is a logical separation: the NvimTypesMap tests are deterministic in-process tests, while the process/RPC tests require an external Neovim executable.

If the objection is that all tests should actually run Nvim, then the old tests didn't do this. Do you propose we just remove the unit tests entirely?

[justinmk]

The unit/integration distinction is not intended to be fast versus slow. It is a logical separation: the NvimTypesMap tests are deterministic in-process tests, while the process/RPC tests require an external Neovim executable.

the only useful distinction is fast vs slow. nothing else matters

[alsi-lawr]

I don't personally think the existing TestMessageDeserialization is better, every test starts with Test*, which is redundant since it's in a test project.

that's completely fine, and again, this repo isn't going to have zillions of tests so it doesn't matter.

If it were f# I'd just write actual english scenario names like

that's BDD format. it is awkward to reference in discussions, code comments, etc, because it's just a free-form name, the test doesn't have an "identifier".

it also seems to encourage no restraint whatsoever when people decide test names.

I agree Given_When_Then is overkill here, but TestMessageDeserialization is too vague. It names the mechanism rather than the concrete expectation, and the Test prefix is redundant. Something like EmbeddedNvimRequestReturnsExpectedResponse is still a normal identifier, just a more useful one.

I also wasn't suggesting we use the standard should-style f# test naming, I was just saying that's a standard in f# when you can have backticked plain-English identifiers.

I don't mind the name being free-form if it makes the failure immediately clear.

TestMessageDeserialization identifies the general area, but not the exact expectation. A plain-English name can still be searched for and referenced, while giving more useful context when it fails.

Names can still be kept concise through review.

[justinmk]

Something like EmbeddedNvimRequestReturnsExpectedResponse is still a normal identifier, just a more useful one.

sure, i just assumed the "Test" prefix was some sort of expectation of xUnit or whatever. if it's not, then no problem.

btw, is xUnit the .NET default/builtin test framework, i.e. not a 3P dependency?

I don't mind the name being free-form if it makes the failure immediately clear.

in practice that almost never matters. what ends up happening is people vomit out paragraphs of descriptions, to solve a problem that never really existed. when a test fails, the assert is what usually matters.

and meanwhile, the long test names cause permanent, ongoing suffering from noisy, over-verbose CI logs. so that's a permanent, ongoing cost to solve the uncommon case.

so everyone does these push-ups to solve the uncommon case, when it could all just be avoided.

most of the industry just got this wrong.

Names can still be kept concise through review.

yep, but not worth adding a dependency.

[alsi-lawr]

Something like EmbeddedNvimRequestReturnsExpectedResponse is still a normal identifier, just a more useful one.

sure, i just assumed the "Test" prefix was some sort of expectation of xUnit or whatever. if it's not, then no problem.

btw, is xUnit the .NET default/builtin test framework, i.e. not a 3P dependency?

xUnit/NUnit/TUnit/MSTest.SDK are all viable. XUnit/NUnit/MSTest.SDK all get support from microsoft, with MSTest.SDK only recently having a modernisation from microsoft to unbind it from its legacy vs days, but I think that's their official internally released one as of 2024. xUnit is a mature independent test platform that Microsoft officially support and contribute to regularly, and it's under the .NET Foundation, so it's not "3rd party" as much as it is independent. xUnit is just a more terse default that people stick to, NUnit is based off of JUnit, and TUnit is a recent super fast parallelised test library.

[justinmk]

yeah, MS changes the landscape a lot, so even if we try to stick with the "default", that rug could be pulled at any time. so xUnit is fine

the only remaining sticking point is:

Do you propose we just remove the unit tests entirely?

I suggest just leaving it as "Test". don't try to carefully separate "unit" vs "integ". it's a waste of time.

there is no scenario where the tests in this repo should ever be "non-deterministic", nor should they take more than 30s (maybe 60s) in total. and if that happens we should ask why, and fix it, rather than saying "oh it's integ, no big deal".