Remove test delays (#5579)

* Changing __indexBuildCompletionCallbackForTests callback to serverStartComplete * Improving serverStartComplete callback to avoid production unhandled promise rejection * Add test to check inexistence of unhandled promise rejection on server fail * Removing some hooks delays * Removing delay after reconfigureServer * Improving code style
parse-community · May 14, 2019 · 893f1d3 · 893f1d3
1 parent 2f161c2
commit 893f1d3
Show file tree

Hide file tree

Showing 11 changed files with 143 additions and 106 deletions.
diff --git a/spec/CloudCode.spec.js b/spec/CloudCode.spec.js
@@ -453,13 +453,13 @@ describe('Cloud Code', () => {
     Parse.Cloud.afterSave('AfterSaveTest', function(req) {
       const obj = new Parse.Object('AfterSaveProof');
       obj.set('proof', req.object.id);
-      obj.save();
+      obj.save().then(test);
     });
 
     const obj = new Parse.Object('AfterSaveTest');
     obj.save();
 
-    setTimeout(function() {
+    function test() {
       const query = new Parse.Query('AfterSaveProof');
       query.equalTo('proof', obj.id);
       query.find().then(
@@ -472,7 +472,7 @@ describe('Cloud Code', () => {
           done();
         }
       );
-    }, 500);
+    }
   });
 
   it('test afterSave ran on created object and returned a promise', function(done) {
@@ -729,15 +729,15 @@ describe('Cloud Code', () => {
     Parse.Cloud.afterDelete('AfterDeleteTest', function(req) {
       const obj = new Parse.Object('AfterDeleteProof');
       obj.set('proof', req.object.id);
-      obj.save();
+      obj.save().then(test);
     });
 
     const obj = new Parse.Object('AfterDeleteTest');
     obj.save().then(function() {
       obj.destroy();
     });
 
-    setTimeout(function() {
+    function test() {
       const query = new Parse.Query('AfterDeleteProof');
       query.equalTo('proof', obj.id);
       query.find().then(
@@ -750,7 +750,7 @@ describe('Cloud Code', () => {
           done();
         }
       );
-    }, 500);
+    }
   });
 
   it('test cloud function return types', function(done) {

diff --git a/spec/EnableExpressErrorHandler.spec.js b/spec/EnableExpressErrorHandler.spec.js
@@ -17,47 +17,45 @@ describe('Enable express error handler', () => {
         masterKey: masterKey,
         serverURL: serverUrl,
         enableExpressErrorHandler: true,
-        __indexBuildCompletionCallbackForTests: promise => {
-          promise.then(() => {
-            expect(Parse.applicationId).toEqual('anOtherTestApp');
-            const app = express();
-            app.use('/parse', parseServer);
+        serverStartComplete: () => {
+          expect(Parse.applicationId).toEqual('anOtherTestApp');
+          const app = express();
+          app.use('/parse', parseServer);
 
-            server = app.listen(12667);
+          server = app.listen(12667);
 
-            app.use(function(err, req, res, next) {
-              next;
-              lastError = err;
-            });
+          app.use(function(err, req, res, next) {
+            next;
+            lastError = err;
+          });
 
-            request({
-              method: 'PUT',
-              url: serverUrl + '/classes/AnyClass/nonExistingId',
-              headers: {
-                'X-Parse-Application-Id': appId,
-                'X-Parse-Master-Key': masterKey,
-                'Content-Type': 'application/json',
-              },
-              body: { someField: 'blablabla' },
+          request({
+            method: 'PUT',
+            url: serverUrl + '/classes/AnyClass/nonExistingId',
+            headers: {
+              'X-Parse-Application-Id': appId,
+              'X-Parse-Master-Key': masterKey,
+              'Content-Type': 'application/json',
+            },
+            body: { someField: 'blablabla' },
+          })
+            .then(() => {
+              fail('Should throw error');
             })
-              .then(() => {
-                fail('Should throw error');
-              })
-              .catch(response => {
-                const reqError = response.data;
-                expect(reqError).toBeDefined();
-                expect(lastError).toBeDefined();
+            .catch(response => {
+              const reqError = response.data;
+              expect(reqError).toBeDefined();
+              expect(lastError).toBeDefined();
 
-                expect(lastError.code).toEqual(101);
-                expect(lastError.message).toEqual('Object not found.');
+              expect(lastError.code).toEqual(101);
+              expect(lastError.message).toEqual('Object not found.');
 
-                expect(lastError.code).toEqual(reqError.code);
-                expect(lastError.message).toEqual(reqError.error);
-              })
-              .then(() => {
-                server.close(done);
-              });
-          });
+              expect(lastError.code).toEqual(reqError.code);
+              expect(lastError.message).toEqual(reqError.error);
+            })
+            .then(() => {
+              server.close(done);
+            });
         },
       })
     );

diff --git a/spec/FilesController.spec.js b/spec/FilesController.spec.js
@@ -46,7 +46,6 @@ describe('FilesController', () => {
     const logController = new LoggerController(new WinstonLoggerAdapter());
 
     reconfigureServer({ filesAdapter: mockAdapter })
-      .then(() => new Promise(resolve => setTimeout(resolve, 1000)))
       .then(() => new Parse.File('yolo.txt', [1, 2, 3], 'text/plain').save())
       .then(
         () => done.fail('should not succeed'),

diff --git a/spec/ParseLiveQueryServer.spec.js b/spec/ParseLiveQueryServer.spec.js
@@ -158,12 +158,10 @@ describe('ParseLiveQueryServer', function() {
           classNames: ['Yolo'],
         },
         startLiveQueryServer: true,
-        __indexBuildCompletionCallbackForTests: promise => {
-          promise.then(() => {
-            expect(parseServer.liveQueryServer).not.toBeUndefined();
-            expect(parseServer.liveQueryServer.server).toBe(parseServer.server);
-            parseServer.server.close(() => done());
-          });
+        serverStartComplete: () => {
+          expect(parseServer.liveQueryServer).not.toBeUndefined();
+          expect(parseServer.liveQueryServer.server).toBe(parseServer.server);
+          parseServer.server.close(done);
         },
       });
     });
@@ -181,16 +179,14 @@ describe('ParseLiveQueryServer', function() {
         liveQueryServerOptions: {
           port: 22347,
         },
-        __indexBuildCompletionCallbackForTests: promise => {
-          promise.then(() => {
-            expect(parseServer.liveQueryServer).not.toBeUndefined();
-            expect(parseServer.liveQueryServer.server).not.toBe(
-              parseServer.server
-            );
-            parseServer.liveQueryServer.server.close(() => {
-              parseServer.server.close(() => done());
-            });
-          });
+        serverStartComplete: () => {
+          expect(parseServer.liveQueryServer).not.toBeUndefined();
+          expect(parseServer.liveQueryServer.server).not.toBe(
+            parseServer.server
+          );
+          parseServer.liveQueryServer.server.close(
+            parseServer.server.close.bind(parseServer.server, done)
+          );
         },
       });
     });

diff --git a/spec/ParseServer.spec.js b/spec/ParseServer.spec.js
@@ -6,6 +6,8 @@ const MongoStorageAdapter = require('../lib/Adapters/Storage/Mongo/MongoStorageA
 const PostgresStorageAdapter = require('../lib/Adapters/Storage/Postgres/PostgresStorageAdapter')
   .default;
 const ParseServer = require('../lib/ParseServer').default;
+const path = require('path');
+const { spawn } = require('child_process');
 
 describe('Server Url Checks', () => {
   let server;
@@ -62,20 +64,38 @@ describe('Server Url Checks', () => {
     }
     const newConfiguration = Object.assign({}, defaultConfiguration, {
       databaseAdapter,
-      __indexBuildCompletionCallbackForTests: promise => {
-        promise.then(() => {
-          parseServer.handleShutdown();
-          parseServer.server.close(err => {
-            if (err) {
-              done.fail('Close Server Error');
-            }
-            reconfigureServer({}).then(() => {
-              done();
-            });
+      serverStartComplete: () => {
+        parseServer.handleShutdown();
+        parseServer.server.close(err => {
+          if (err) {
+            done.fail('Close Server Error');
+          }
+          reconfigureServer({}).then(() => {
+            done();
           });
         });
       },
     });
     const parseServer = ParseServer.start(newConfiguration);
   });
+
+  it('does not have unhandled promise rejection in the case of load error', done => {
+    const parseServerProcess = spawn(
+      path.resolve(__dirname, './support/FailingServer.js')
+    );
+    let stdout;
+    let stderr;
+    parseServerProcess.stdout.on('data', data => {
+      stdout = data.toString();
+    });
+    parseServerProcess.stderr.on('data', data => {
+      stderr = data.toString();
+    });
+    parseServerProcess.on('close', code => {
+      expect(code).toEqual(1);
+      expect(stdout).toBeUndefined();
+      expect(stderr).toContain('MongoNetworkError:');
+      done();
+    });
+  });
 });
diff --git a/spec/PostgresInitOptions.spec.js b/spec/PostgresInitOptions.spec.js
@@ -28,16 +28,18 @@ function createParseServer(options) {
     const parseServer = new ParseServer.default(
       Object.assign({}, defaultConfiguration, options, {
         serverURL: 'http://localhost:12666/parse',
-        __indexBuildCompletionCallbackForTests: promise => {
-          promise.then(() => {
+        serverStartComplete: error => {
+          if (error) {
+            reject(error);
+          } else {
             expect(Parse.applicationId).toEqual('test');
             const app = express();
             app.use('/parse', parseServer.app);
 
             const server = app.listen(12666);
             Parse.serverURL = 'http://localhost:12666/parse';
             resolve(server);
-          }, reject);
+          }
         },
       })
     );

diff --git a/spec/helper.js b/spec/helper.js
@@ -150,10 +150,13 @@ const reconfigureServer = changedConfiguration => {
         defaultConfiguration,
         changedConfiguration,
         {
-          __indexBuildCompletionCallbackForTests: indexBuildPromise =>
-            indexBuildPromise.then(() => {
+          serverStartComplete: error => {
+            if (error) {
+              reject(error);
+            } else {
               resolve(parseServer);
-            }, reject),
+            }
+          },
           mountPath: '/1',
           port,
         }

diff --git a/spec/index.spec.js b/spec/index.spec.js
@@ -295,30 +295,28 @@ describe('server', () => {
         appId: 'aTestApp',
         masterKey: 'aTestMasterKey',
         serverURL: 'http://localhost:12666/parse',
-        __indexBuildCompletionCallbackForTests: promise => {
-          promise.then(() => {
-            expect(Parse.applicationId).toEqual('aTestApp');
-            const app = express();
-            app.use('/parse', parseServer.app);
-
-            const server = app.listen(12666);
-            const obj = new Parse.Object('AnObject');
-            let objId;
-            obj
-              .save()
-              .then(obj => {
-                objId = obj.id;
-                const q = new Parse.Query('AnObject');
-                return q.first();
-              })
-              .then(obj => {
-                expect(obj.id).toEqual(objId);
-                server.close(done);
-              })
-              .catch(() => {
-                server.close(done);
-              });
-          });
+        serverStartComplete: () => {
+          expect(Parse.applicationId).toEqual('aTestApp');
+          const app = express();
+          app.use('/parse', parseServer.app);
+
+          const server = app.listen(12666);
+          const obj = new Parse.Object('AnObject');
+          let objId;
+          obj
+            .save()
+            .then(obj => {
+              objId = obj.id;
+              const q = new Parse.Query('AnObject');
+              return q.first();
+            })
+            .then(obj => {
+              expect(obj.id).toEqual(objId);
+              server.close(done);
+            })
+            .catch(() => {
+              server.close(done);
+            });
         },
       })
     );
@@ -332,7 +330,8 @@ describe('server', () => {
         appId: 'anOtherTestApp',
         masterKey: 'anOtherTestMasterKey',
         serverURL: 'http://localhost:12667/parse',
-        __indexBuildCompletionCallbackForTests: promise => {
+        serverStartComplete: error => {
+          const promise = error ? Promise.reject(error) : Promise.resolve();
           promise
             .then(() => {
               expect(Parse.applicationId).toEqual('anOtherTestApp');

diff --git a/spec/support/FailingServer.js b/spec/support/FailingServer.js
@@ -0,0 +1,10 @@
+#!/usr/bin/env node
+
+const ParseServer = require('../../lib/index').ParseServer;
+
+ParseServer.start({
+  appId: 'test',
+  masterKey: 'test',
+  databaseURI:
+    'mongodb://doesnotexist:27017/parseServerMongoAdapterTestDatabase',
+});
diff --git a/src/Options/index.js b/src/Options/index.js
@@ -177,7 +177,7 @@ export interface ParseServerOptions {
   /* Live query server configuration options (will start the liveQuery server) */
   liveQueryServerOptions: ?LiveQueryServerOptions;
 
-  __indexBuildCompletionCallbackForTests: ?() => void;
+  serverStartComplete: ?(error: ?Error) => void;
 }
 
 export interface CustomPagesOptions {

diff --git a/src/ParseServer.js b/src/ParseServer.js
@@ -80,7 +80,7 @@ class ParseServer {
       cloud,
       javascriptKey,
       serverURL = requiredParameter('You must provide a serverURL!'),
-      __indexBuildCompletionCallbackForTests = () => {},
+      serverStartComplete,
     } = options;
     // Initialize the node client SDK automatically
     Parse.initialize(appId, javascriptKey || 'unused', masterKey);
@@ -100,11 +100,21 @@ class ParseServer {
     const hooksLoadPromise = hooksController.load();
 
     // Note: Tests will start to fail if any validation happens after this is called.
-    if (process.env.TESTING) {
-      __indexBuildCompletionCallbackForTests(
-        Promise.all([dbInitPromise, hooksLoadPromise])
-      );
-    }
+    Promise.all([dbInitPromise, hooksLoadPromise])
+      .then(() => {
+        if (serverStartComplete) {
+          serverStartComplete();
+        }
+      })
+      .catch(error => {
+        if (serverStartComplete) {
+          serverStartComplete(error);
+        } else {
+          // eslint-disable-next-line no-console
+          console.error(error);
+          process.exit(1);
+        }
+      });
 
     if (cloud) {
       addParseCloud();