8000 app.setup() should be async · Issue #853 · feathersjs/feathers · GitHub
[go: up one dir, main page]

Skip to content

app.setup() should be async #853

@zaro

Description

@zaro

I have feathers app that uses sequelize for storage. For the tests, I use sqlite database, which is removed before the test starts, and is created in sequelize.js file as generated by feathers generate service. This all worked fine until the number of models increased beyond 10 ( the number is dependent on how fast the machine is). Now every time I run the full test suite the first couple of tests fail because the DB sync hasn't completed yet and it happens so that the first tests that get ran are the last tables being created.

As a simple workaround , I changed the before of app.test.js(it's the first test being executed) like so :

--- a/core/test/app.test.js
+++ b/core/test/app.test.js
@@ -18,7 +18,7 @@ const getUrl = pathname => url.format({
 describe('Feathers application tests', () => {
   before(function(done) {
     this.server = app.listen(port);
-    this.server.once('listening', () => done());
+    this.server.once('listening', () => setTimeout( ()=> done(), 1000));
   });

This fixes it for now until the number of models grows of the test is executed on a slower machine.
The real problem is that app.listen() calls app.setup(), but doesn't wait for it to complete before firing the listening event. The database is created in app.setup and can take some time.

I think app.setup() should be async and app.listen() should wait for it to complete, or else there will always be possibility for race conditions between setup() and service requiring the setup() to complete before it's functional.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions

      0