Add TypeScript definition

[iffy]

This is picking up on Issue #256. (Aside: Seems weird to make a different PR instead of tracking the conversation and changes in one place. GitHub seems to not allow for otherwise, though)

I've added a "test" (i.e. compile a TypeScript test file). But... I think that the TypeScript test file needs to exercise every function (and function variation) in order to be useful. And my problem is two-fold:

1. I don't know how to use Tap yet (I'm just picking it up), so I don't know how to exercise every part of it.
2. I don't know how to exercise the test-failing parts (e.g. tap.fail("this failed")) without also making the tests fail.

Any ideas?

[iffy]

Also, I think the type definition is a good start, but I think it's not completely correct. For example, beforeEach has this definition:

beforeEach(fn: (cb: () => any) => Promise | void):void

but in reality, the callback optionally receives a done thinger, right? http://www.node-tap.org/api/#tbeforeeachfunction-done-

[iffy]

I've also submitted this as a PR to the PR: dlmanning#3 I feel like I'm being confusing, but I don't know the right process.

[isaacs]

To exercise t.fail, you can create a new Test object and mess around with it. I think it might be worth holding off until v11, since some interface stuff may move around (and get a lot more organized in the process) when I migrate to using es classes.

…

On Tue, Jun 27, 2017 at 11:09 Matt Haggard ***@***.***> wrote: I've also submitted this as a PR to the PR: dlmanning#3 <dlmanning#3> I feel like I'm being confusing, but I don't know the right process. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#381 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAkRyIdwVQZNpFwaGDkJZdN0pKk9f7cks5sIUVQgaJpZM4OG-2B> .

[iffy]

@isaacs Over lunch I thought, "what I need is a repository of code that exercises all of Tap." If only such a repository existed... like in the test/ directory. I'm going to figure out how to leverage the existing tests to assert that the type definitions are correct.

[iffy]

The most recent commit is a bunch of failing tests (step 1). If it's a less-than-an-hour exercise, I'll try to make them pass (step 2). Otherwise, I'll wait for v11.

[iffy]

Here's a summary of the outstanding TypeScript compilation errors. Some of the problems can be fixed in a variety of ways. @isaacs, if you have an opinion on how to fix these, I'd like guidance. Otherwise, I'll use my best judgment.

Variable Reuse

These errors are for pieces of code like:

let arg = 'some string';
arg = arg.split(' ');

TypeScript doesn't like you to change arg from a string to an array of strings.

How to fix it

1. Change the JS code so that no variables are reused.
2. Add // @ts-ignore to the failing lines

Instances

test/runner-jobs.js(22,5): error TS2322: Type 'string | { [x: string]: any; jobs: number; bail: boolean; reporter: string; color: boolean; }' is not assignable to type '{ [x: string]: any; jobs: number; }'.
  Type 'string' is not assignable to type '{ [x: string]: any; jobs: number; }'.
test/runner-output-file.js(20,5): error TS2322: Type 'string[]' is not assignable to type 'string'.
test/spawn-failures.js(25,3): error TS2322: Type 'Error' is not assignable to type 'boolean'.
test/spawn-failures.js(44,3): error TS2322: Type 'Error' is not assignable to type 'boolean'.
test/test.js(59,7): error TS2322: Type '"timeout and handles tests too timing dependent for Travis"' is not assignable to type 'boolean'.
test/test.js(61,7): error TS2322: Type '"timeout and handles tests rely on sinals windows cannot do"' is not assignable to type 'boolean'.
test/test.js(67,7): error TS2322: Type '"skip segv on windows"' is not assignable to type 'boolean'.
test/test.js(69,7): error TS2322: Type '"skip segv on CI"' is not assignable to type 'boolean'.
test/test.js(74,7): error TS2322: Type '"sigterm handling test does not work on 0.10"' is not assignable to type 'boolean'.
test/test.js(76,7): error TS2322: Type '"sigterm handling is weird on windows"' is not assignable to type 'boolean'.
test/test.js(116,5): error TS2322: Type 'string[]' is not assignable to type 'string'.
test/test/spawn-failures.js(25,3): error TS2322: Type 'Error' is not assignable to type 'boolean'.
test/test/spawn-failures.js(34,3): error TS2322: Type 'Error' is not assignable to type 'boolean'.

Ordering and Optionals

node-tap is really flexible when it comes to argument order in many functions. I'm struggling to define the declarations that can handle that flexibility.

How to fix it

1. Figure out how to describe what each function can accept
2. Add // @ts-ignore to the failing lines

Instances

test/runner-timeout.js(47,11): error TS2345: Argument of type '{ skip: string | boolean; }' is not assignable to parameter of type 'string'.
test/test/subtest-only-options.js(4,8): error TS2345: Argument of type '{ skip: boolean; some: string; diagnostic: boolean; }' is not assignable to parameter of type 'string'.

Type tests

Some of the tests are asserting that the type of arguments is correct (e.g. you must pass a number to plan() and the test tries to pass a string to test that)

How to fix it

1. Port portions of node-tap to TypeScript :)
2. Add // @ts-ignore to the failing lines

Instances

test/test-test.js(90,13): error TS2345: Argument of type '"foo"' is not assignable to parameter of type 'number'.
test/test-test.js(103,38): error TS2345: Argument of type '"not a function"' is not assignable to parameter of type 'Callback'.
test/test/unfinished.js(6,16): error TS2345: Argument of type '"some event that never happens"' is not assignable to parameter of type 'Signals'.

Misc

Is it okay to assign a number to process.env.TAP_BUFFER?

test/test.js(10,1): error TS2322: Type '1' is not assignable to type 'string'.

Where is Test.pipe defined? I can't find it in the docs or in the code. Potential fix: add this to the test-only type declaration.

test/test/bailout.js(34,3): error TS2339: Property 'pipe' does not exist on type '{ pushedEnd: boolean; jobs: any; subtests: any[]; pool: any; queue: string[]; noparallel: boolean...'.

I think maybe this is a bug in the node typings:

test/test/segv.js(17,32): error TS2559: Type '"utf8"' has no properties in common with type '{ encoding?: string; mode?: string; flag?: string; }'.

IDK

I'm not proficient enough with TypeScript to know how to fix these :)

How to fix it

1. Learn what's wrong and fix it
2. Add // @ts-ignore to the failing lines

Instances

test/runner-nyc-args.js(18,26): error TS2307: Cannot find module 'nyc/package.json'.
test/runner-timeout.js(20,14): error TS2339: Property '__coverage__' does not exist on type 'Global'.
test/runner-version.js(17,25): error TS2307: Cannot find module '../package.json'.
test/test-args.js(25,9): error TS2360: The left-hand side of an 'in' expression must be of type 'any', 'string', 'number', or 'symbol'.
test/test/rejects.js(36,13): error TS2345: Argument of type '10' is not assignable to parameter of type 'Test | PromiseLike<Test>'.
test/test/rejects.js(72,13): error TS2345: Argument of type '10' is not assignable to parameter of type 'Test | PromiseLike<Test>'.
test/runner-jobs.js(24,42): error TS2345: Argument of type '(string | { [x: string]: any; jobs: number; bail: boolean; reporter: string; color: boolean; })[]' is not assignable to parameter of type 'string | string[]'.
  Type '(string | { [x: string]: any; jobs: number; bail: boolean; reporter: string; color: boolean; })[]' is not assignable to type 'string[]'.
    Type 'string | { [x: string]: any; jobs: number; bail: boolean; reporter: string; color: boolean; }' is not assignable to type 'string'.
      Type '{ [x: string]: any; jobs: number; bail: boolean; reporter: string; color: boolean; }' is not assignable to type 'string'.

[iffy]

Looks like the master branch builds are failing, too. Once master passes tests, I suspect these tests will pass too.

[iffy]

@isaacs ping? This approach (using node-tap's own tests to test the typescript definition file) would work, no?

[isaacs]

@iffy As discussed in #387, the only way presently to make this work would be to rewrite all of the tests in typescript, or rewrite the entire lib in typescript, and neither are agreeable to me. It's too much of a hassle to add a transpilation step.

[iffy]

@isaacs this branch uses the existing javascript tests to assert that the type definition works. You do not need to rewrite all the test to TypeScript (because of the allowJS and checkJS options in tsconfig.json).

How does this work with your criteria:

1. defines a method that doesn't exist - compilation will fail
2. defines return/arg/property types incorrectly - compilation will fail
3. fails to define a public method/property - compilation will fail
4. does not require rewriting the module or all tests in typescript - correct; this does not

I don't actually use this lib anymore, but if you'd like, I can prepare examples of each of these to prove it.

[isaacs]

@iffy If those criteria are met, then yeah, that's great. But it'll definitely need to be rebased onto latest master. I'll mention it in #387, maybe someone else can pick it up if you're no longer interested. Thanks.