Skip to content
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

Async test setup #235

Closed
wants to merge 4 commits into from
Closed

Conversation

KoryNunn
Copy link

@KoryNunn KoryNunn commented Jan 8, 2016

As per #234

Attempting to call additional tests after some delay causes tape to hang.

This branch resolves by the addition of the endWaitTime option.

By default, there is no wait time, and tape acts as usual.

Setting endWaitTime to a value eg 50 causes tape to wait 50ms between t 'end' events before emitting 'done'. This allows tests that get set up asynchronously after other tests have completed, to run.

endWaitTime is a suggestion, there are probably better names for it.

@yoshuawuyts
Copy link

sound reasonable to me; implementation looks good

@ghost
Copy link

ghost commented Jan 9, 2016

This would make tests very timing sensitive, which sounds like a fragile feature. What's so bad about moving async operations inside the test:

test('whatever', function (t) {
  asyncSetup(function () {
     // ...
  })
})

or moving async setup to a pre-test:

test('setup', function (t) {
  asyncSetup(function () { t.end() })
})

test('whatever', function (t) {
  // ...
})

With similar techniques for sub-tests. Is the more about subtests in async handlers?

@KoryNunn
Copy link
Author

I'm trying to use tape for integration testing, where each set of tests need to run in series, with a db flush in between.

The issue is, because that flush takes some time, tape decides that the tests must be finished.

I'm not a huge fan of this "just wait a bit" solution, and I'm also investigating other solutions.

I'll give the pre-test implementation a go, my only reservation would be having a test that only serves to prevent tape from finishing.

@KoryNunn
Copy link
Author

@substack I gave the setup test implementation a go, but it doesn't work, because the results only attempt to complete after a t.end, so no tests are run after the setup test begins.

This PR (#236) does work for that case, and also solves the test added in the current PR.

@Raynos
Copy link
Collaborator

Raynos commented Jan 16, 2016

I think this is a bad idea.

There are better approaches.

@KoryNunn Please look at https://www.npmjs.com/package/tape-cluster

@KoryNunn
Copy link
Author

@Raynos I agree, please see #236 and let me know what you think.

@KoryNunn KoryNunn closed this Jan 18, 2016
@KoryNunn KoryNunn deleted the async-test-setup branch January 18, 2016 02:39
@nathanmarks
Copy link

FWIW, this is how I am handling async test setup:

I have the following helper function that runs my DB migrations on an sqlite DB.

You could also run seeds here, or any other async setup. (this is only for a single test, but with some more code you can get something setup that can handle all sorts of flushing/resetting/seeding etc)

import Test from 'blue-tape';
import Bookshelf from 'lib/db';
import KnexCleaner from 'knex-cleaner';

export function TestWithDB (name, callback, testFn = Test) {
  return testFn(name, t => {
    return Bookshelf.knex.migrate.latest()
      .then(() => callback(t))
      .then(() => Bookshelf.knex.migrate.rollback())
      .then(() => KnexCleaner.clean(Bookshelf.knex));
  });
}

TestWithDB.only = function (name, callback) {
  return TestWithDB(name, callback, Test.only);
};

Test.onFinish(() => {
  return Bookshelf.knex.destroy();
});

and to use it....

TestWithDB('Tool runner integration', t => {
  // DB is setup and can be used
  // using blue-tape so return a promise here
});

@mLuby
Copy link

mLuby commented Jul 19, 2018

Surprised there isn't an test.onStart analog to test.onFinish. I'd think @substack's legitimate concerns over timing sensitivity/fragility would apply equally to both. Since onFinish is allowed why not onStart (which I'm happy to PR 😇).

  • Regarding "moving async operations inside the test", you may want the setup to occur only once. Yes there are ways around it but they're gross.
  • Regarding "moving async setup to a pre-test", this breaks when .only is used.

(Also I can open this as a new issue if preferred…)

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

Successfully merging this pull request may close these issues.

5 participants