Make test context optionally type aware for TypeScript #1298

mmkal · 2017-03-05T19:48:53Z

This sets out to solve the problem highlighted in #1291

Usage example: see comment further down.

Also add 'contextualize' function to main module exports, which will allow seeding the context, and return a test function which will give the context property the correct type.

novemberborn

Clever!

Would it be possible to do:

function contextualize<T>(getContext: () => T): ava.ITest<T> {

(Without repeating the { context } bit?

Perhaps @despairblue, @ivogabe, @SamVerschueren could provide input as well? 😇

novemberborn · 2017-03-07T18:16:00Z

.gitignore

@@ -3,3 +3,6 @@ node_modules
 coverage
 bench/.results
 types/generated.d.ts
+
+# VS Code configuration
+.vscode


You should add this to your machine's .gitignore, rather than adding it in this repo.

Sure. I didn't want to do that initially because I have other repos that I want .vscode files checked in for, but maybe I can use .git/info/exclude instead.

novemberborn · 2017-03-07T18:23:41Z

types/base.d.ts

-export function test(run: ContextualTest): void;
-export function test(name: string, run: Macros<ContextualTestContext>, ...args: any[]): void;
-export function test(run: Macros<ContextualTestContext>, ...args: any[]): void;
+export interface ITest<T> {


Let's just use Test<T>.

Can do - I think my C# is showing 😳

ivogabe · 2017-03-07T18:58:01Z

types/base.d.ts

 export type Test = (t: TestContext) => PromiseLike<void> | Iterator<any> | Observable | void;
-export type ContextualTest = (t: ContextualTestContext) => PromiseLike<void> | Iterator<any> | Observable | void;
+export type GenericTest<T> = (t: GenericTestContext<T>) => PromiseLike<void> | Iterator<any> | Observable | void;


Add export type ContextualTest = GenericTest<{ context: any }>;, otherwise this might break some users. Same for ContextualCallbackTest

ivogabe · 2017-03-07T19:01:16Z

Overall this looks good. Could you add some comments to make.js? It might be a bit hard to read now.

mmkal · 2017-03-07T19:18:36Z

@novemberborn re whether we could do:

function contextualize<T>(getContext: () => T): ava.ITest<T> { ... }

Maybe what I could do is export two types - Test<T> (renamed from ITest) and GenericContextualTest<T> which could be defined by

export type GenericContextualTest<T> = Test<{ context: T }>;

The name GenericContextualTest isn't great, but I'll see what I can do about avoiding a clash with ContextualTest when implementing @ivogabe's suggestion.

Anyway, this way people wouldn't have to constantly repeat the context: ... part. I'll add something along those lines soon and update.

Gets rid of a lot of repeated definitions, and should make it easier to make the whole thing properly generic.

mmkal · 2017-03-08T05:19:39Z

@novemberborn @ivogabe updated with suggested changes, and improved a couple of other things:
I renamed ITest as suggested, but I am now using it as the interface for all test-like functions - and they are all now optionally generic. Plus, types/generated.d.ts is now about 25% smaller because each test-like function now implements this single interface rather than having its declaration spread over four lines.

Usage example:

import * as ava from 'ava'

function contextualize<T>(getContext: () => T): ava.ContextualTestFunction<T> {
    ava.test.beforeEach(t => {
        Object.assign(t.context, getContext());
    });

    return ava.test;
}

const test = contextualize(() => ({ foo: 'bar' }));

test.beforeEach(t => {
    t.context.foo = 123; // error:  Type '123' is not assignable to type 'string'
});

test.after.always.failing.cb.serial('very long chains are properly typed', t => {
    t.context.fooo = 'a value'; // error: Property 'fooo' does not exist on type '{ foo: string }'
});

test('an actual test', t => {
    t.deepEqual(t.context.foo.map(c => c), ['b', 'a', 'r']); // error: Property 'map' does not exist on type 'string'
});

In the above example, contextualize is a generic method, and infers the type of the object passed to it to be { foo: string }. By using the test method returned instead of the vanilla ava one, we can take advantage of type annotations.

This would allow people to extend ava with things like their favourite assertion/mocking libraries, and have strongly typed, convenient test setups:

import MyClass from '../MyClass';
import calc = require('npm-calculator');
import chai = require('chai');
import sinon = require('sinon');
import contextualize from './ava-contextualize'; // same contextualize method as above as a module

const test = contextualize(() => {
    const myClass = new MyClass();
    const sandbox = sinon.sandbox.create();
    const expect = chai.expect;
    return { myClass, sandbox, expect };
});

test.afterEach(t => t.sandbox.restore());

test('times uses npm-calculator multiply', t => {
    t.context.sandbox.stub(calc, 'multiply').returns(42);
    const sixTimesNine = t.context.myClass.times(6, 9);
    t.context.expect(calc.multiply).to.be.calledOnce;
    t.context.expect(sixTimesNine).to.equal(42);
});

mmkal · 2017-03-09T16:01:23Z

Here's a gist of the generated types file, since it can't be seen on the PR.
@novemberborn @despairblue @ivogabe I'd love to get this merged as I'm planning to use it in my current project. Sorry about so many small commits - I'd be happy for it to go in as it is now, if it looks OK to you.

ivogabe

Thanks for the gist, the generated file looks good to me. Using underscores is not very common, but is probably the best way to go. With caps we could have some problems, for instance testAfterEach could then be after.each and afterEach. Though I think that we should rename the first part of the names, since types should start with an upper case letter. So maybe one of these:

Rename TestFunction(Core) to Register(Base), and test_** to Register_**
Rename TestFunction(Core) to AVA(Core), and test_** to AVA_.

ivogabe · 2017-03-09T17:40:54Z

types/make.js

+// Returns the declarations for the 'test' function and core types
+function testFunctionDeclarations() {
+	return (
+`export default test;


Move this to base.d.ts

If I do, base.d.ts will no longer compile because ContextualTestFunction and TestFunction (or Register and RegisterContextual once they're renamed) don't exist in base.d.ts.

Or do you think that doesn't matter so much because base.d.ts isn't used for anything except seeding generated.d.ts?

That's not an issue, base.d.ts is indeed only used by the generation script to write generated.d.ts.

ivogabe · 2017-03-09T17:41:23Z

types/base.d.ts

-export type ContextualCallbackTest = (t: ContextualCallbackTestContext) => void;
+export type GenericCallbackTest<T> = (t: GenericCallbackTestContext<T>) => void;
+
+interface Context<T> { context: T }


Add export

ivogabe · 2017-03-09T17:51:37Z

types/base.d.ts

-export function test(run: ContextualTest): void;
-export function test(name: string, run: Macros<ContextualTestContext>, ...args: any[]): void;
-export function test(run: Macros<ContextualTestContext>, ...args: any[]): void;
+interface TestFunctionCore<T> {


Can we think of a better name here? Technically this is not the function that does the tests, it only registers a test. So maybe something like RegisterBase<T>, and rename TestFunction Register<T>?

And add export here

Also rename test_* interfaces to Register_*.

mmkal · 2017-03-10T14:09:24Z

@ivogabe updated with the exports and renames you suggested. Gist also updated.

sindresorhus · 2017-03-11T08:58:09Z

This should be documented somewhere, probably here: https://github.com/avajs/ava/blob/master/docs/recipes/typescript.md

mmkal · 2017-03-11T15:54:03Z

@sindresorhus added the above usage example.

ivogabe · 2017-03-11T16:18:21Z

Looks good to me, I've tested it on one of my projects and it worked just fine.

This does however not work with TypeScript 2.0, 2.1 is required (because of the use of T['skip']). 2.1 was released december last year. But that's probably not an issue since AVA is not 1.0 yet.

sindresorhus · 2017-03-12T17:27:32Z

readme.md

@@ -578,6 +578,8 @@ Keep in mind that the `beforeEach` and `afterEach` hooks run just before and aft

 Remember that AVA runs each test file in its own process. You may not have to clean up global state in a `after`-hook since that's only called right before the process exits.

+### Test context


### => ####

sindresorhus · 2017-03-14T12:51:59Z

Thank you @mmkal. This is a great improvement! :)

mmkal · 2017-03-14T17:38:08Z

Thanks @sindresorhus @novemberborn @ivogabe ! Does it get auto-published to NPM? Or is it done ad-hoc?

sindresorhus · 2017-03-14T17:47:10Z

@mmkal No, releases are manually published. Was planning to do a new one next week (It's a lot of work to manually test and write release notes).

mmkal added 4 commits March 5, 2017 14:13

Make test context (optionally) type aware.

c55923a

Also add 'contextualize' function to main module exports, which will allow seeding the context, and return a test function which will give the context property the correct type.

lint: Use property shorthand

2b9857f

Get rid of contextualize; it's not necessary

e030d81

Make TestContext generic rather than Contextual.

32bba3d

mmkal mentioned this pull request Mar 7, 2017

Subsequent variable declarations must have the same type when trying to type t.context [TypeScript] #1291

Closed

novemberborn requested changes Mar 7, 2017

View reviewed changes

ivogabe reviewed Mar 7, 2017

View reviewed changes

mmkal added 9 commits March 7, 2017 20:54

Reintroduce types ContextualTest and ContextualCallbackTest

9a3d5f0

Add newline to end of gitignore

6de1948

Use types instead of namespaces.

f8d9229

Gets rid of a lot of repeated definitions, and should make it easier to make the whole thing properly generic.

Make the default export a properly defined type

9fb9e17

Make default export a narrowing of a generic type

44d8d90

More flexible generics

de19e57

Even more flexible generics

d3a5981

Get rid of dead code

909670e

Lint fixes

fa35b08

mmkal added 3 commits March 8, 2017 00:20

Another trailing space somehow snuck in

fe7d3c9

More make.js comments & switch to interfaces.

297f723

Make Context<T> an interface

d11918a

mmkal changed the title ~~[WIP] Make test context (optionally) type aware.~~ Make test context (optionally) type aware. Mar 8, 2017

Make TestFunction an interface

dfb9d62

ivogabe suggested changes Mar 9, 2017

View reviewed changes

Rename TestFunction to Register.

e8148a0

Also rename test_* interfaces to Register_*.

Add usage example to TypeScript recipe doc.

584092b

Add TypeScript >= 2.1 requirement to recipe.

fae6023

sindresorhus reviewed Mar 12, 2017

View reviewed changes

sindresorhus changed the title ~~Make test context (optionally) type aware.~~ Make test context optionally type aware Mar 12, 2017

sindresorhus changed the title ~~Make test context optionally type aware~~ Make test context optionally type aware for TypeScript Mar 12, 2017

Make 'Test context' a lower-status heading.

b6e1125

novemberborn approved these changes Mar 13, 2017

View reviewed changes

sindresorhus merged commit 50ad213 into avajs:master Mar 14, 2017

renovate-bot mentioned this pull request Apr 5, 2017

Update dependency ava to version 0.19.1 singapore/gl-got#20

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make test context optionally type aware for TypeScript #1298

Make test context optionally type aware for TypeScript #1298

mmkal commented Mar 5, 2017 •

edited

Loading

novemberborn left a comment

novemberborn Mar 7, 2017

mmkal Mar 7, 2017

novemberborn Mar 7, 2017

mmkal Mar 7, 2017

ivogabe Mar 7, 2017

ivogabe commented Mar 7, 2017

mmkal commented Mar 7, 2017

mmkal commented Mar 8, 2017 •

edited

Loading

mmkal commented Mar 9, 2017

ivogabe left a comment •

edited

Loading

ivogabe Mar 9, 2017

mmkal Mar 9, 2017 •

edited

Loading

ivogabe Mar 9, 2017

ivogabe Mar 9, 2017

ivogabe Mar 9, 2017

mmkal commented Mar 10, 2017

sindresorhus commented Mar 11, 2017

mmkal commented Mar 11, 2017

ivogabe commented Mar 11, 2017

sindresorhus Mar 12, 2017

sindresorhus commented Mar 14, 2017

mmkal commented Mar 14, 2017

sindresorhus commented Mar 14, 2017

		@@ -578,6 +578,8 @@ Keep in mind that the `beforeEach` and `afterEach` hooks run just before and aft

		Remember that AVA runs each test file in its own process. You may not have to clean up global state in a `after`-hook since that's only called right before the process exits.

		### Test context

Make test context optionally type aware for TypeScript #1298

Make test context optionally type aware for TypeScript #1298

Conversation

mmkal commented Mar 5, 2017 • edited Loading

novemberborn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ivogabe commented Mar 7, 2017

mmkal commented Mar 7, 2017

mmkal commented Mar 8, 2017 • edited Loading

mmkal commented Mar 9, 2017

ivogabe left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmkal Mar 9, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmkal commented Mar 10, 2017

sindresorhus commented Mar 11, 2017

mmkal commented Mar 11, 2017

ivogabe commented Mar 11, 2017

Choose a reason for hiding this comment

sindresorhus commented Mar 14, 2017

mmkal commented Mar 14, 2017

sindresorhus commented Mar 14, 2017

mmkal commented Mar 5, 2017 •

edited

Loading

mmkal commented Mar 8, 2017 •

edited

Loading

ivogabe left a comment •

edited

Loading

mmkal Mar 9, 2017 •

edited

Loading