[RFC] Case based / BDD organization strategy for tests

[thescientist13]

Type of Change

• New Feature Request
• Documentation / Website
• Improvement / Suggestion
• Bug
• Other (please clarify below)

Summary

As being discussed in #52 , it looks like there is a case to be made for having a deliberate plan to write, manage and organize unit tests as the project scales and more configuration / APIs become available and we don't create a giant single file of unit tests.

Also wort tracking this conversation about testing configuration / setup: #53 (review)

Details

In general, the main goals of this suggestion are to help address / solve some of the observed and encountered issues with growing out our unit tests

• Reduce overlapping concerns / duplicate testing
• Lack of clarity structure for how to setup, organize, and group tests
• Ensure test run times are manageable
• Provide confidence in good coverage of all key code / hot paths through unit testing

One project I've found that has a good approach to unit test build tools is webpack. They organize their tests by cases, which I think is a great paradigm for us, in terms of thinking but organization.

I propose we do something similar, with goal of having clearly articulated test cases that can avoid the need to have redundant tests and ensure tests can focus specifically on the features / changes they are meant to cover.

Cases could be based on packages maybe?

test/
  fixtures/  // same as it is now
  unit/   // if we want to unit tests individual files?
  cases/
    cli/
       build.default.spec.js   
       build.error-handling.spec.js
       build.nested-directories.js
       build.config.workspaace.spec.js
       build.config.dev-server.spec.js

Common things can still be done TestSetup still. Maybe there could be a way to ensure a common set of tests pass each time somehow? I think tests are an exception though when it comes to keeping things DRY.

Open to ideas, but I think at the very least the organizational aspect of something like this (not necessarily the technical aspects) will be very useful not only for us as maintainers, but also contributors.

Note: might be good to do this and #50 and #67 together

[hutchgrant]

I agree, but the way I see it there are 2 types of tests that have to be done:

1. tests on the default template
2. tests on the user workspace

each of these cases, may need to test 1 or both those types of tests.

the user workspace test should include nested directories. So for example, if we're testing a config, I want to test all userworkspace related things, including nested directories.

I'm definitely in favor of splitting it up but there's a number of shared tests.

[thescientist13]

Yeah, I think what you're aiming and what I'm decribing are essentially having some sort of low level shared "smoke tesys" we can easily run across multiple describe / files. Like something in addition to TestSetup that could be pulled into each test. (it could also maybe include the repeated before / after we use too)?

I think as long as each test case clearly own its own specific assertions and the setup / configuration is still in the hands of each test, and we keep this utility fairly low level and a convenience for just easily sanity testing a suite of tests before moving onto case specific it, then that seems like a good balance to work towards. 👍

ex.

const TestSetup = require('./setup');
const TestUtils = require('./utils');

describe('building greenwood with custom userWorkspace directory', () => {
  before(async () => {
    // custom userland config
     const config = {
       userWorkspace: 'www'
     };

     // get a custom configured context
     CONTEXT = await setup.init(config);

     // setup a testing environment
     setup = new TestSetup(CONTEXT);
     setup.setupWorkspace();  // fs.mkdirSync, etc
    
     // run the tool
     await setup.run(['./packages/cli/index.js', 'build']);
  });

  it('should pass all smoke tests', async () => {
    TestUtils.runSmokeTest(CONTEXT);
  });

  it('should test some other things...', async () => {
    // maybe it would be good to update our CONTRIBUTING.md to add best practices, tips for writing unit tests for the project?
  });

  after(async() => {
    setup.teardownWorkspace();  // fs.remove, etc
  });

});

IDK. My main concern is really limiting what happens in this line

TestUtils.runSmokeTest(CONTEXT);

The value of testing is recreating the users experience using our tool, too much abstraction could keep us too far removed from that and miss bugs / make assumptions not specific to how a user would actually be doing it.

[thescientist13]

Pretty good watch on general testing strategies
https://www.youtube.com/watch?v=cB5WZgMwdpE

[thescientist13]

PRs to watch for coverage here

• an empty greenwood.config.js breaks the build (due to devServer?) #66
• try and get testing of HTML output lost in Make front-matter label var optional #58