Default testcluster size

[jdom]

• Default to 2 silos being started in the TestCluster.
  This is to spot serialization issues that don't occur in a 1 silo deployment that just does deep copying.
• Demoted some very slow tests from BVT to Functional.
  They are still being run by our CI server, just not every time a dev wants to run all BVTs or for every pull request.
  This should shave off a couple of minutes from our BVT run

[jason-bragg]

If our BVT test runs are getting too long, I'd prefer we look into speeding them up via optimizations or parallelization rather than reducing test coverage.

[veikkoeeva]

As a bit of a outsider comment, the contents of this PR look OK, but I'd raise the same concern as @jason-bragg about demoting. Maybe it is worth nothing here too cleaning up tests has come into discussion from time to time.

@jason-bragg About parallelization and perhaps optimizations, not trying to hijack this thread, I wrote a short spike at #1803 with one the goals having tests run in parallel (in every environment) skipping those that cannot be run (e.g. no ZooKeeper installed.

[jdom]

regardless of parallelization i wouldn't expect hat any single test that we use for TDD (which should by design be very fast) take more than a few milliseconds. I did not go that far as being overly aggressive, but the ones I demoted here were taking more than 15seconda each (some of them a minute).
Again, not sure what makes them Bvts as opposed to Functionals (in the past we added tests just as functionals when they were very slow).
Parallelization definitely makes sense, but making Bvts run several minutes faster now as opposed to waiting a few months until we build support in xunit to parallelize tests within collections (which is not supported out of the box) I believe it's very much worth it. I run BVTs several times a day. The only reason I don't run them more often (even though I certainly prefer that when doing TDD) is exactly because it takes a long time. Once we build support or parallelization, we should still do a sweep and promote several of our Functionals to BVTs.
If you still think we should not demote them, I'll undo the changes here and only keep them in my local repo, as I deem them very valuable. Hopefully I will always remember to undo those changes before submitting other PRs

[jdom]

BTW, I could tag them with something like "SlowBVT" and have our CI servers run them, we just don't pay that tax on every single time we run BVTs while developing

[veikkoeeva]

@jdom I'm what one could describe as an occasional contributor so you should take my opinion as such, from this perspective I'm OK with the changes as it unlocks you. I'm familiar with the pain you experience, which is one of the reasons for a more concrete proposal at #1803.

I prefer to run through the whole suite before submitting a PR for review, and othertimes too I usually just run through the whole suite of relative tests, most likely the ones tagged with SQL Server or MySQL. I would like these tests to be fast and I don't see an immediate reason on why they couldn't be run in low tens of milliseconds each – unless stress testing excluding a cluster setup when it is warranted. It looks to me even the cluster setup could be very fast with explicit list of assemblies, perhaps caching them, creating clusters in parallel etc. that we seem to get in the future.

This goes into another discussion already, but I linked a Stackoverflow question in #1803 that specifically asks to run theories in parallel across backends. It was not recommended and the arrangement taken was much like in #1803. To me it looks constructor injection of the SUT on the class having the concrete test classes would be the way, but XUnit.NET doesn't do that out-of-the-box, the authors tell elsewhere they didn't want to include a IoC system in the tests. The arrangement for runnign tests I spiked gets close to this but one has to duplicate the theories (basically copy and paste) to pass-through shims (i.e. SQL Server, MySQL, Azure Table Storage etc.) delegating the parameters to the actual test class. It looks to me this could work without malleating XUnit.NET to something else. Instead of theories it might be possible to generate (fuzzed) test input like IntelliTest with FsCheck.

[jason-bragg]

I don't disagree with the idea of fast BVTs. Every large project with automated testing eventually gets to the point where build and build verification times becomes an issue, but when this happens the first step should not, imo, be to start dropping test coverage. I'm ok with a SlowBVT tag if we add some sort of warning to the bvts that help us detect slow running tests so that we have an opportunity to improve the test performance prior to more extreme actions. Some of the tests removed here are streaming tests that I've not even looked into making faster. Maybe they could be fixed..

I suggest the following:

• Set a target max run time for BVT tests.
• Log warnings for tests that run too long.
• Make a practice out of not adding any new tests that run too long.
• Mark all existing tests that run to long as SlowBvts.
• Create work items to revisit slow bvt tests to see if we can bring them into line.

[jdom]

Marked the demoted tests as SlowBVT and updated scripts so that they run in each PR.

We can set up a task for later on to look at any SlowBVT with the potential to improve its speed. This will probably be low priority as the ROI at that point is not super high (some of these have to take long to run, since they are asserting the lack of action, although the timeout times could be made shorter, but not instant, hence you might shave off a few seconds of very long running tests).
Regarding automatically logging warnings, that would be nice, not sure whether xUnit allows it. I consider this much lower priority, and basically just manually demoted the super-slow tests for now. When running tests in VS, you can actually group by duration and see the Slow tests.