Mark Non Unit Tests

[mjbvz]

Some of the existing tests are not unit tests. We need a way to distinguish these from the true unit tests in the project.

Rough guide for identifying non-unit tests in a first pass:

• Takes longer than 250ms to run (should be decreased later).
• Non-deterministic.
• Would fail in a sandbox without file system or network access.
• Depends on test execution order.
• UI automation.

Goal is to have a set of unit tests that can be run from inside visual studio directly, quickly and with complete confidence. This will allow detecting regressions more quickly.

At this stage, it may be easier to mark the true unit tests instead of marking the non-unit tests.
Related to #646 .

[mousetraps]

Perhaps a better demarcation would be UI Tests and non-UI tests (this would simply be based on the VSTestHost attribute, and also - for the most part - should satisfy the "runs quickly" property because we don't have to wait for UI.)

Re: "UnitTest" and "IntegrationTest" categories - I'm not sure how valuable this is given the constraints of our codebase. Many of the longer-running tests are the closest we can get to a 'unit test' due to some of the constraints of Node / VS, and I think that - in our codebase - just running things that would be classified as "unit tests" provides a false sense of security.

[mjbvz]

You are correct that unit tests will not provide 100% end-to-end confidence, but that's not their job. The unit test should run quickly, continuously, and reliably to drive development and catch simple errors. Once all the unit tests pass, then you can kick off the 5-20min full test suite or a more targeted subset of integration tests to make sure no regressions have been introduced. As it stands, there aren't very many tests currently that test individual units of the code.

Marking UI tests also makes sense since they are slower, have more dependencies, and may be less reliable than the rest of our tests. Perhaps everything without a category should just be assumed to be a normal IntegrationTest then? But I still think there is value is marking the unit tests separately and using them as a tool for development and code maintenance.

[mousetraps]

I think we both agree on what might make a good unit test in an ideal world. However, unfortunately we are not building an application in isolation - we are building on top of platforms that we have no control over, and oftentimes that's the most important functionality to test - at the whim of 20yo VS interfaces, of of Node.js, npm, etc. So the definition proposed is far too strict for our taste.

A unit test is, quite simply, something that tests a single assumption about a unit of functionality in an application.

And, of course, we don't expect unit tests to provide 100% end-to-end confidence...

But they should cover enough scenarios such that they are actually useful in catching errors, otherwise why bother running the tests at all? For instance, the "leg" tests may run really fast (pun totally intended) but the head and arm tests might not be shining examples of a unit test, and discounting those tests because they don't meet some arbitrary criteria provides a false sense of security. Now, we could ask "well, why don't we make all of our other tests that follow this definition and provide enough coverage"... and again, awesome in principle, and we can discuss it, but the Q is how it weighs against our other priorities.

As a thought experiment, let's take something that looks simple on the surface. Let's say we want to write unit tests for "snippets" that actually follow this strict definition - how might we go about doing that? Turns out it's not so easy. 😃

So, in addition to running quickly, continuously, and reliably... we want the tests to help drive development, and in order to do so, it's not sufficient that the tests run quickly, continuously, and reliably - they also need to provide value.

So okay, great - we want to provide value, but we also want them to run quickly... how can we do that? Well during development you don't actually have to run the full test suite locally - I run the portion of the test suite that's actually relevant to the area that I'm working on and leave it to CI to catch anything else that I missed. So if I'm working on npm, I'll run the npm tests, etc. Right now, I'll just do this via search filters, but categories would be incredibly useful here. Even better if they match up with the categories we have on GitHub so it's easier for people unfamiliar with the codebase to know which tests to run!

Does this make sense?

[mjbvz]

I have a different take on unit tests. Perhaps its just terminology but a unit test should test a unit of code. I don't really think unit tests are actually that helpful as tests, and they certainly cannot ensure the user experience will be good, but they are very helpful development tools, either when writing new code using a test driven development approach, or when touching existing code. That's their value. If each unit of code works, we may have more confidence in the end to end flow, but the unit tests alone cannot guarantee this.

I'm coming at this having used ncrunch and that style of tool for development. If a single test takes 5 seconds, has state, or touches the file system/network, that type of development just doesn't work. That's why I believe marking unit tests has value and that's the intent of this change. CI validation and normal development would still run all the existing tests just fine, but a developer would be more easily able to scope the contentiously running set of tests to the true unit tests.

[mousetraps]

Unit of code / functionality / work is all just saying the same thing in different ways.

I have a friend, he's a parent, and he once said "There have been a lot of books written about parenting... but none of them have been written about my child."

Just as there is no "true" way to raise a child, there is no "true" way to test an application. There's a set of guiding principles, but ultimately you choose what works on a project-by-project basis. As I explained, the proposed definition is too strict and the investment involved to actually reach that ideal doesn't provide enough value for this project.

[mousetraps]

Again, let's definitely consider other ways of improving our test infrastructure, but let's move from thinking about what is best to what is best for this project.

[mjbvz]

Closing this for now since @mousetraps and I talked at length offline on this area and agree this isn't a top priority.

This series of posts sums up my take on unit testing pretty well (and links to a lot of great other resources on the subject).

On the testing front going forward, first step is certainly to get AppVeyor passing again (#731) and then documenting what tests should be run for a given change to make contributions easier. We'll also be looking into #718 as a low cost way to get failing tests on people's radars.