-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Add automated regression testing against old TS versions #1752
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Copying the useful bits from my dup issue: The simplest approach might be extending CI's It'd be great as a next step (followup issue?) to also make use of CodeCov's report merging to combine unit test coverage from all of them. #2516 is an example PR where code under test changes behavior on the TypeScript version. |
The thing that makes this difficult is ensuring that the rules that have variant functionality on TS versions will have to pass on the other TS versions (this is the same problem that makes #2464 difficult) To make this work we'll have to extend and customise the core ESLint RuleTester and design a config system for it. There's a lot of codecov features we should totally leverage more of - I've just never bothered to read the docs 😅 |
To start, can that be as simple as checking the TypeScript version at Jest runtime when constructing the
ruleTester.run('some-rule', rule, {
// ...
});
if (util.isAboveTsVersion('3.9')) {
ruleTester.run('some-rule typescript@>=3.9', rule, {
// ...
});
} else {
ruleTester.run('some-rule typescript@<3.9', rule, {
// ...
});
}
// (may need to add overrides or some logic if any TS releases had unusual version numbers...)
export const isAboveTsVersion = (version: string) =>
semver.satisfies(
ts.version,
`>= ${version}.0 || >= ${version}.1-rc || >= ${version}.0-beta`,
{
includePrerelease: true,
},
); Are there other testing features needed? |
It should be pretty trivial for us to extend the rule tester and add a property to the config object. It'd set a good precedence for doing a similar thing with ESLint version Something like: type RunTests<TMessageIds extends string, TOptions extends Readonly<unknown[]>> = MakeSomeChangesToTheType<TSESLint.RunTests<TMessageIds, TOptions>>;
class RuleTester extends RuleTester {
run<TMessageIds extends string, TOptions extends Readonly<unknown[]>>(
name: string,
rule: TSESLint.RuleModule<TMessageIds, TOptions>,
testsReadonly: RunTests<TMessageIds, TOptions>,
): void {
const tests = { ...testsReadonly };
// standardize the valid tests as objects
tests.valid = tests.valid.map(test => {
if (typeof test === 'string') {
return {
code: test,
};
}
return test;
});
const filter = test => isValidTsVersion(test.tsVersion);
tests.valid = tests.valid.filter(filter);
tests.invalid = tests.valid.filter(filter);
}
} |
Following up on original suggestion, I just pushed a suggestion to |
I think in the next major we need to consider filling DefinitelyTyped's example and bump our minimum TS version as well? We support a super old one and it's getting to be a but out of date at this point! DT supports 4.0 and above for a few more days (at which point 4.1 will be EOL) |
Proposal: let's support the same time range as DT? I don't see any particular reason to go older. |
BREAKING CHANGE: Bumps the minimum supported range and removes handling for old versions. Ref #1752
#1698 introduced a regression for TS<3.8.x (#1746).
This wasn't caught by the type checker, nor was it caught by the tests because both were using TS3.8.
It was a simple mistake, but one that shouldn't have happened.
We should add an integration test in order to make sure that this doesn't happen again.
We cannot run builds against older versions of TS, because our code relies upon property types etc added by the latest versions, but we can easily run tests against the old versions.
I don't think we need to test every version we support, but at least testing against the previous one or two minors and the minimum supported version should be good enough.
Regressions that would have been caught:
export * as ns
#1698 used an enum value added in TS3.8.The text was updated successfully, but these errors were encountered: