Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Don't call getPreEmitDiagnostics in tests #2769

Merged
merged 7 commits into from
May 15, 2017
Merged

Don't call getPreEmitDiagnostics in tests #2769

merged 7 commits into from
May 15, 2017

Conversation

ajafff
Copy link
Contributor

@ajafff ajafff commented May 15, 2017

PR checklist

  • Addresses an existing issue: #0000
  • New feature, bugfix, or enhancement
    • Includes tests
  • Documentation update

Overview of change:

The diagnostics were ignored anyway and are not required for typechecking.
Creating a program and therefore enabling typechecked rules is now done when a tsconfig.json file is placed in the folder next to tslint.json.
That allows us to get rid of linterOptions.typeCheck in tslint.json. It's still supported but deprecated in case someone used it in his tests.

Is there anything you'd like reviewers to focus on?

This reduces the time needed to run the rule tests by 50%.
I'll also submit a PR that enables typechecked rules when passing --project (without --type-check) on CLI. That will result in a 20% speedup when linting this project.

CHANGELOG.md entry:

[develop] testing rules with type information is enabled when a tsconfig.json is found next to tslint.json

Copy link
Contributor

@adidahiya adidahiya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

generally 👍 for the idea

src/test.ts Outdated
@@ -84,6 +86,11 @@ export function runTest(testDirectory: string, rulesDirectory?: string | string[
};
compilerOptions = ts.parseJsonConfigFileContent(config, parseConfigHost, testDirectory).options;
}
// TODO remove in v6.0.0
if (tslintConfig !== undefined && tslintConfig.linterOptions !== undefined && tslintConfig.linterOptions.typeCheck === true) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

linterOptions.typecheck was never public; we can remove it immediately. #1445 (comment)

@@ -0,0 +1 @@
{}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

an empty file looks pretty weird; can we at least make it extend some common tsconfig file? perhaps located at test/rules/test-tsconfig.json?

Copy link
Contributor Author

@ajafff ajafff May 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't want to extend because that adds unnecessary overhead. I just added compilerOptions.module to have some config in the file.
Edit: target caused some problems, that's why I used module

@ajafff
Copy link
Contributor Author

ajafff commented May 15, 2017

Updated as suggested. Tests pass.
Added documentation.

Similar PR for runner.ts will be ready tomorrow morning.

Once this PR and #2762 are merged, I will refactor test.ts to only create the program once per directory.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants