gui-cs / Terminal.Gui

Cross Platform Terminal UI toolkit for .NET
MIT License
9.74k stars 695 forks source link

Enable unit tests to run in parallel #3007

Open tig opened 1 year ago

tig commented 1 year ago

Running the unit tests on a fast machine can take upwards of 3 minutes. Runs in Github Action can take even longer. This is annoying and slows development.

The reason the unit tests are forced to run serially ("parallelizeTestCollections": false,) is primarily because Application is a singleton.

However, over time I've worked hard to make as many unit tests (and code) independent of Application.

We should create another xunit test project (Low-level UnitTests?) that has "parallelizeTestCollections": true, and move any test that is independent of Application (or other statics) there.

tig commented 10 months ago

From this issue, via @dodexahedron:

We should consider leveraging Nunit as part of addressing THIS Issue.

As I'm working on Color, I'm also re-working most of the existing tests. Some are just getting unrolled into parameterized tests, some are getting expanded, some are getting re-worked, some are getting removed, and some are getting replaced, as appropriate - a lot of the same sort of work I've been doing for TGD, just XUnit-flavored over here.

Thank you. I strongly suspect the work you are doing will serve as a great "how to do it RIGHT" for everyone else (including me). Thank you. Thank you.

  • Would you mind terribly if I included NUnit as well, if or when something would be significantly better or easier? That comes like 1/4 from personal preference and majority just from the fact that it's way more robust and expressive. No worries if you'd rather I not.

I have no real love of xUnit. I used NUnit in another project and it was fine too. I'd like @BDisp and @tznind to chime in on this. I'm NOT a fan of using TWO test frameworks if we can avoid it. So, if you really think NUnit is signficantly better, and it makes your life significantly better, I would support a transition. But @BDisp and @tznind have to fully support this too because it will be a LOT of work.

  • Do you have a naming convention you're married to for test methods you'd like me to stick to? I try to be pretty descriptive with them, and thus far I've been mostly following along with what existing tests look like, with the exception of I usually don't also include the name of the class itself in the test method, since the test fixture already has that.
  • Be clear, concise, and complete. Clear means human readable and descriptive of the theory. Concise means terse (no class-name). Complete means no surprises pop when you actually look at the test code.
  • Use underscores where you'd use a space. Use camel-case for names of classes/methods/properties that are camel-cased.

What else?

  • Do you mind if, when it is important to do so, that I make something that was private be internal, for test purposes? I think that situation should typically be pretty rare, but just throwing it out there.

I actually encourage it. However, please add XML docs to any such element and have the docs say why it's internal (e.g. "This method is internal to support unit testing.). I wish there was a way of declaring a member "Make this internal for unit testing, but for god's sake don't let it be used anywhere else but this class."

All sounds good to me and are pretty much how I operate anyway, so yay.

As for the test framework, it's not terribly important to me. TGD uses NUnit, and part of the work I've been doing there is actually updating the code from NUnit 2-era assertions to NUnit 4, which is current.

There are various libraries meant to work with multiple frameworks which can add in nice features without changing it all. One example would be FluentAssertions which basically just let's you use fluent-style method chains to make more expressive assertion statements, which is a similar idea to what NUnit has natively, but even takes the concept further and is of course an optional add-on (thus non-breaking to existing tests). And then libraries like the one I used for those combinatorial tests also exist in various forms. That one was pretty commonly.mentioned and recommended around the net, which is why I picked that one. That and it's an active project, which is also good.

In the end, a test harness is just code, and the framework is just syntactic sugar to reduce boilerplate.

However (and keep in mind I'm not pushing for a change, I'm just delivering information)....

Here are some key features native to NUnit that are just damn nice:

  • Test ordering by simple Order attributes. Useful when you want to ensure that more basic, common core code is tested first, so time isn't wasted running a bunch of tests that will ultimately fail because some key dependency is broken.
  • Pre-conditions using the Assume statement (rather than Assert. These are for the setup phase of a test fixture or test method, when you want to guarantee or prove a specific set of starting conditions for the actual test to be considered valid. Failing these gives the test case an Indeterminate result, which is more informative that something other than the code under test is causing the problem.
  • More types of assertions built-in
  • Parallel test execution with control granularity all the way down to the individual generated cases if parameterized tests.
  • Combinatorial, pairwise, and sequential options for generation of test cases for parameterized tests. Sequential is handy and really cuts down on the need to write test case generator methods. The other two exist in a limited form in the extension I pulled in for xUnit.
  • Much much much much MUCH better documentation. My god, the xUnit "documentation" is sorely lacking.
  • Automatic expansion of enum parameters to a test case for each defined enum value, via this: [Values] (yep that's literally it), and that on top of the other rich parameterization goodness.
  • Nice informational attributes for indicating what type or method a given test fixture or test method is testing. This is handy for informational purposes and some tools can also make use of it.
  • Attributes to control which test methods or test fixtures are selected by the test runner based on the platform of the machine running the test (so you can test platform-specific code without messy conditional compilation or extra code in tests to handle that stuff)
  • Threading control, such as forcing a test or fixture to run on a single thread, such as when testing certain things around statics.
  • Tons more, but I'm feeling self-conscious like I'm advertising for it or something lol. Just have a look at the UnitTests project in TGD. I make use of everything I've mentioned here all over the place and more. 😅

Though one thing I will say that is intended somewhat as a directed point in support of switching: It's mostly a Find/Replace job to switch, as everything in the existing tests is there with the same syntax but just different attribute names.

Haha but enough about that. I'm gonna get back to work on Color and its tests. 😅

(This message clearly brought to you by NUnit.) 😆

dodexahedron commented 10 months ago

Some of this has been mentioned, but I'm just putting more words and detail to stuff.

These are some options available:

Split up the test project

First idea, which is independent of whether test framework changes or not, is to split the test project up into 2 projects - one for anything that actually requires firing up an Application, and the other for everything else.

Pros:

Cons:

Multiple test runner configurations

Second idea is also independent of any others and thus can be used with or without any other strategy, regardless of framework.

Different configurations can be used within a single test project in any framework through use of things such as the --filter argument to dotnet test among other ways of selecting subsets of tests to run. With XUnit, that would be easiest to achieve by using Traits, such as the "Category" Trait I've applied to the new tests I've written. Trait is kind of a generic catch-all attribute for whatever arbitrary classification you want to give a test. NUnit's direct equivalent is Property, though things like Category are also formal attributes in NUnit.

Using this option, test classes or methods that require Application setup and teardown could be marked as such, or those that don't could be marked, or both. Then, either test runner configuration files (such as different copies of the xunit.runner.json file), --filter arguments, or other means can be used to run each subset of tests with the desired/required settings.

This is a very flexible option and isn't all-or-nothing, and I think it's a prudent strategy to implement no matter which framework is in use or what other ideas we choose to implement.

Pros:

Cons (these are super-minor, but I wanted to come up with something):

tig commented 10 months ago

Love.

How about

Then, we can all start migrating tests.

??

dodexahedron commented 10 months ago

Hahaha

Well... I'm trying to take it slow.

As I mentioned in the other issue, tests are just code and anything doable in one framework is doable in another or even without a test framework, if one writes the necessary code.

For right now, and for the PR I'll put in for the Color stuff, I'm still using XUnit. Fewer changes are still a nice thing to strive for when possible.

But yes, if we want to split anything up, vis-a-vis project or framework, that action plan sounds great.

I'd offer that, if we do use NUnit for stuff, I'll be happy to shoulder most or all of the burden for conversion, since I catalyzed that initiative. 😅

dodexahedron commented 10 months ago

And also, another key is I don't want to distract from work on the actual library itself. Hence why I'm happy to lighten the load if any changes to framework are settled on.

tig commented 10 months ago

I'd offer that, if we do use NUnit for stuff, I'll be happy to shoulder most or all of the burden for conversion, since I catalyzed that initiative. 😅

Oops. I mis-typed above. I wrote xunit, but mean nunit. I've edited it to be correct.

dodexahedron commented 10 months ago

Huh. I didn't even notice that and my brain filled in "NUnit" the first time, anyway. 😅