New testing system for rules

[jkillian]

Implements #620 and provides a solution to #532.

Documentation for this feature will come in a separate PR. The code is still a little bit messy, but I'd like to get a second set of eyes on it for ideas to make it nicer.

In brief, src/test.ts and src/test/* contain the logic/code for parsing the markup of the new testing system. test/rule-tester contains the tests for this code. test/rules contains one directory for every rule which contains the rule's tests written in the new markup style.

[adidahiya]

copyright block

[adidahiya]

how about "... the expected output in .linttest files" (or whatever we end up naming the extension)

[adidahiya]

looks pretty great overall.

I'm wondering if we can make the structure of the linttests simpler. All the .linttest files are called test.ts.linttest and are adjacent to exactly one tslint.json file. Maybe we could do something like TypeScript's fourslash tests?

for example, test/rules/no-require-imports.ts.linttest:

//// tslint.json
{
  "rules": {
    "no-require-imports": true
  }
}

//// linttest
var lib = require('lib'); // failure
          ~~~~~~~~~~~~~~             [no-require]

let lib2 = require('lib2'); // failure
           ~~~~~~~~~~~~~~~             [no-require]

import {l} from 'lib';

var lib3 = load('not_an_import');

var lib4 = lib2.subImport;

var lib5 = require('lib5'), // failure
           ~~~~~~~~~~~~~~~             [no-require]
    lib6 = require('lib6'), // failure
           ~~~~~~~~~~~~~~~             [no-require]
    lib7 = 700;

import lib8 = require('lib8'); // failure
              ~~~~~~~~~~~~~~~             [no-require]

import lib9 = lib2.anotherSubImport;

[no-require]: require() style import is forbidden

this would flatten the directories. you could still have multiple files for a rule, e.g. test/rules/variable-name/allow-leading-underscore.ts.linttest. thoughts?

[jkillian]

I'm wondering if we can make the structure of the linttests simpler. All the .linttest files are called test.ts.linttest and are adjacent to exactly one tslint.json file. Maybe we could do something like TypeScript's fourslash tests?

Interesting idea. There are a few cases where this isn't true (no-unused-variable/default, no-unused-variable/react, and some others probably). I don't really feel that this would be a bad idea, but I don't really feel like it would be a huge benefit either. What do you think?

[adidahiya]

yeah, I don't see it as a huge benefit either; the tradeoffs are minor. the current structure does support cases like test/rules/no-unused-variable/ nicely, where you don't have to repeat the contents of tslint.json across your tests. let's keep it how it is.

somewhat relatedly, any ideas for a better directory name than _comprehensive? It's a little misleading because "comprehensive" implies testing all the rules. Perhaps _integration?

[adidahiya]

why do you need to include build/test/lint.js here?

[adidahiya]

it seems to still work if I remove it.

[adidahiya]

filed a separate issue for updating documentation, which should be handled next: #941

[adidahiya]

👍