ci: Add test retry logic for flaky tests (#9218)

dplewis · web-flow · commit 9fd707092eeb · 2024-08-11T14:24:50.000+02:00
diff --git a/spec/Idempotency.spec.js b/spec/Idempotency.spec.js
@@ -45,6 +45,10 @@ describe('Idempotency', () => {
     });
   });
 
+  afterEach(() => {
+    jasmine.DEFAULT_TIMEOUT_INTERVAL = process.env.PARSE_SERVER_TEST_TIMEOUT || 10000;
+  });
+
   // Tests
   it_id('e25955fd-92eb-4b22-b8b7-38980e5cb223')('should enforce idempotency for cloud code function', async () => {
     let counter = 0;
diff --git a/spec/RegexVulnerabilities.spec.js b/spec/RegexVulnerabilities.spec.js
@@ -16,8 +16,13 @@ const emailAdapter = {
 const appName = 'test';
 const publicServerURL = 'http://localhost:8378/1';
 
-describe('Regex Vulnerabilities', function () {
-  beforeEach(async function () {
+describe('Regex Vulnerabilities', () => {
+  let objectId;
+  let sessionToken;
+  let partialSessionToken;
+  let user;
+
+  beforeEach(async () => {
     await reconfigureServer({
       maintenanceKey: 'test2',
       verifyUserEmails: true,
@@ -38,13 +43,13 @@ describe('Regex Vulnerabilities', function () {
         email: 'someemail@somedomain.com',
       }),
     });
-    this.objectId = signUpResponse.data.objectId;
-    this.sessionToken = signUpResponse.data.sessionToken;
-    this.partialSessionToken = this.sessionToken.slice(0, 3);
+    objectId = signUpResponse.data.objectId;
+    sessionToken = signUpResponse.data.sessionToken;
+    partialSessionToken = sessionToken.slice(0, 3);
   });
 
-  describe('on session token', function () {
-    it('should not work with regex', async function () {
+  describe('on session token', () => {
+    it('should not work with regex', async () => {
       try {
         await request({
           url: `${serverURL}/users/me`,
@@ -53,7 +58,7 @@ describe('Regex Vulnerabilities', function () {
           body: JSON.stringify({
             ...keys,
             _SessionToken: {
-              $regex: this.partialSessionToken,
+              $regex: partialSessionToken,
             },
             _method: 'GET',
           }),
@@ -65,43 +70,43 @@ describe('Regex Vulnerabilities', function () {
       }
     });
 
-    it('should work with plain token', async function () {
+    it('should work with plain token', async () => {
       const meResponse = await request({
         url: `${serverURL}/users/me`,
         method: 'POST',
         headers,
         body: JSON.stringify({
           ...keys,
-          _SessionToken: this.sessionToken,
+          _SessionToken: sessionToken,
           _method: 'GET',
         }),
       });
-      expect(meResponse.data.objectId).toEqual(this.objectId);
-      expect(meResponse.data.sessionToken).toEqual(this.sessionToken);
+      expect(meResponse.data.objectId).toEqual(objectId);
+      expect(meResponse.data.sessionToken).toEqual(sessionToken);
     });
   });
 
-  describe('on verify e-mail', function () {
+  describe('on verify e-mail', () => {
     beforeEach(async function () {
       const userQuery = new Parse.Query(Parse.User);
-      this.user = await userQuery.get(this.objectId, { useMasterKey: true });
+      user = await userQuery.get(objectId, { useMasterKey: true });
     });
 
-    it('should not work with regex', async function () {
-      expect(this.user.get('emailVerified')).toEqual(false);
+    it('should not work with regex', async () => {
+      expect(user.get('emailVerified')).toEqual(false);
       await request({
         url: `${serverURL}/apps/test/verify_email?username=someemail@somedomain.com&token[$regex]=`,
         method: 'GET',
       });
-      await this.user.fetch({ useMasterKey: true });
-      expect(this.user.get('emailVerified')).toEqual(false);
+      await user.fetch({ useMasterKey: true });
+      expect(user.get('emailVerified')).toEqual(false);
     });
 
-    it_id('92bbb86d-bcda-49fa-8d79-aa0501078044')('should work with plain token', async function () {
-      expect(this.user.get('emailVerified')).toEqual(false);
+    it_id('92bbb86d-bcda-49fa-8d79-aa0501078044')('should work with plain token', async () => {
+      expect(user.get('emailVerified')).toEqual(false);
       const current = await request({
         method: 'GET',
-        url: `http://localhost:8378/1/classes/_User/${this.user.id}`,
+        url: `http://localhost:8378/1/classes/_User/${user.id}`,
         json: true,
         headers: {
           'X-Parse-Application-Id': 'test',
@@ -115,18 +120,18 @@ describe('Regex Vulnerabilities', function () {
         url: `${serverURL}/apps/test/verify_email?username=someemail@somedomain.com&token=${current._email_verify_token}`,
         method: 'GET',
       });
-      await this.user.fetch({ useMasterKey: true });
-      expect(this.user.get('emailVerified')).toEqual(true);
+      await user.fetch({ useMasterKey: true });
+      expect(user.get('emailVerified')).toEqual(true);
     });
   });
 
-  describe('on password reset', function () {
-    beforeEach(async function () {
-      this.user = await Parse.User.logIn('someemail@somedomain.com', 'somepassword');
+  describe('on password reset', () => {
+    beforeEach(async () => {
+      user = await Parse.User.logIn('someemail@somedomain.com', 'somepassword');
     });
 
-    it('should not work with regex', async function () {
-      expect(this.user.id).toEqual(this.objectId);
+    it('should not work with regex', async () => {
+      expect(user.id).toEqual(objectId);
       await request({
         url: `${serverURL}/requestPasswordReset`,
         method: 'POST',
@@ -137,7 +142,7 @@ describe('Regex Vulnerabilities', function () {
           email: 'someemail@somedomain.com',
         }),
       });
-      await this.user.fetch({ useMasterKey: true });
+      await user.fetch({ useMasterKey: true });
       const passwordResetResponse = await request({
         url: `${serverURL}/apps/test/request_password_reset?username=someemail@somedomain.com&token[$regex]=`,
         method: 'GET',
@@ -162,8 +167,8 @@ describe('Regex Vulnerabilities', function () {
       }
     });
 
-    it('should work with plain token', async function () {
-      expect(this.user.id).toEqual(this.objectId);
+    it('should work with plain token', async () => {
+      expect(user.id).toEqual(objectId);
       await request({
         url: `${serverURL}/requestPasswordReset`,
         method: 'POST',
@@ -176,7 +181,7 @@ describe('Regex Vulnerabilities', function () {
       });
       const current = await request({
         method: 'GET',
-        url: `http://localhost:8378/1/classes/_User/${this.user.id}`,
+        url: `http://localhost:8378/1/classes/_User/${user.id}`,
         json: true,
         headers: {
           'X-Parse-Application-Id': 'test',
@@ -204,7 +209,7 @@ describe('Regex Vulnerabilities', function () {
         },
       });
       const userAgain = await Parse.User.logIn('someemail@somedomain.com', 'newpassword');
-      expect(userAgain.id).toEqual(this.objectId);
+      expect(userAgain.id).toEqual(objectId);
     });
   });
 });
diff --git a/spec/helper.js b/spec/helper.js
@@ -14,6 +14,7 @@ if (dns.setDefaultResultOrder) {
 jasmine.DEFAULT_TIMEOUT_INTERVAL = process.env.PARSE_SERVER_TEST_TIMEOUT || 10000;
 jasmine.getEnv().addReporter(new CurrentSpecReporter());
 jasmine.getEnv().addReporter(new SpecReporter());
+global.retryFlakyTests();
 
 global.on_db = (db, callback, elseCallback) => {
   if (process.env.PARSE_SERVER_TEST_DB == db) {
@@ -287,7 +288,7 @@ afterEach(function (done) {
 });
 
 afterAll(() => {
-  global.displaySlowTests();
+  global.displayTestStats();
 });
 
 const TestObject = Parse.Object.extend({
diff --git a/spec/support/CurrentSpecReporter.js b/spec/support/CurrentSpecReporter.js
@@ -1,13 +1,36 @@
 // Sets a global variable to the current test spec
 // ex: global.currentSpec.description
 const { performance } = require('perf_hooks');
+
 global.currentSpec = null;
 
-const timerMap = {};
-const duplicates = [];
+/**
+ * Names of tests that fail randomly and are considered flaky. These tests will be retried
+ * a number of times to reduce the chance of false negatives. The test name must be the same
+ * as the one displayed in the CI log test output.
+ */
+const flakyTests = [
+  // Timeout
+  "ParseLiveQuery handle invalid websocket payload length",
+  // Unhandled promise rejection: TypeError: message.split is not a function
+  "rest query query internal field",
+  // TypeError: Cannot read properties of undefined (reading 'link')
+  "UserController sendVerificationEmail parseFrameURL not provided uses publicServerURL",
+  // TypeError: Cannot read properties of undefined (reading 'link')
+  "UserController sendVerificationEmail parseFrameURL provided uses parseFrameURL and includes the destination in the link parameter",
+  // Expected undefined to be defined
+  "Email Verification Token Expiration:  sets the _email_verify_token_expires_at and _email_verify_token fields after user SignUp",
+];
+
 /** The minimum execution time in seconds for a test to be considered slow. */
 const slowTestLimit = 2;
 
+/** The number of times to retry a flaky test. */
+const retries = 5;
+
+const timerMap = {};
+const retryMap = {};
+const duplicates = [];
 class CurrentSpecReporter {
   specStarted(spec) {
     if (timerMap[spec.fullName]) {
@@ -26,20 +49,74 @@ class CurrentSpecReporter {
     global.currentSpec = null;
   }
 }
-global.displaySlowTests = function() {
-  const times = Object.values(timerMap).sort((a,b) => b - a);
+
+global.displayTestStats = function() {
+  const times = Object.values(timerMap).sort((a,b) => b - a).filter(time => time >= slowTestLimit);
   if (times.length > 0) {
     console.log(`Slow tests with execution time >=${slowTestLimit}s:`);
   }
   times.forEach((time) => {
-    if (time >= slowTestLimit) {
-      console.warn(`${time.toFixed(1)}s:`, Object.keys(timerMap).find(key => timerMap[key] === time));
-    }
+    console.warn(`${time.toFixed(1)}s:`, Object.keys(timerMap).find(key => timerMap[key] === time));
   });
   console.log('\n');
   duplicates.forEach((spec) => {
     console.warn('Duplicate spec: ' + spec);
   });
+  console.log('\n');
+  Object.keys(retryMap).forEach((spec) => {
+    console.warn(`Flaky test: ${spec} failed ${retryMap[spec]} times`);
+  });
+  console.log('\n');
 };
 
+global.retryFlakyTests = function() {
+  const originalSpecConstructor = jasmine.Spec;
+
+  jasmine.Spec = function(attrs) {
+    const spec = new originalSpecConstructor(attrs);
+    const originalTestFn = spec.queueableFn.fn;
+    const runOriginalTest = () => {
+      if (originalTestFn.length == 0) {
+        // handle async testing
+        return originalTestFn();
+      } else {
+        // handle done() callback
+        return new Promise((resolve) => {
+          originalTestFn(resolve);
+        });
+      }
+    };
+    spec.queueableFn.fn = async function() {
+      const isFlaky = flakyTests.includes(spec.result.fullName);
+      const runs = isFlaky ? retries : 1;
+      let exceptionCaught;
+      let returnValue;
+
+      for (let i = 0; i < runs; ++i) {
+        spec.result.failedExpectations = [];
+        returnValue = undefined;
+        exceptionCaught = undefined;
+        try {
+          returnValue = await runOriginalTest();
+        } catch (exception) {
+          exceptionCaught = exception;
+        }
+        const failed = !spec.markedPending &&
+            (exceptionCaught || spec.result.failedExpectations.length != 0);
+        if (!failed) {
+          break;
+        }
+        if (isFlaky) {
+          retryMap[spec.result.fullName] = (retryMap[spec.result.fullName] || 0) + 1;
+        }
+      }
+      if (exceptionCaught) {
+        throw exceptionCaught;
+      }
+      return returnValue;
+    };
+    return spec;
+  };
+}
+
 module.exports = CurrentSpecReporter;
diff --git a/spec/support/jasmine.json b/spec/support/jasmine.json
@@ -2,5 +2,5 @@
   "spec_dir": "spec",
   "spec_files": ["*spec.js"],
   "helpers": ["helper.js"],
-  "random": false
+  "random": true
 }

Original file line number	Diff line number	Diff line change
`@@ -2,5 +2,5 @@`
`2`	`2`	`"spec_dir": "spec",`
`3`	`3`	`"spec_files": ["*spec.js"],`
`4`	`4`	`"helpers": ["helper.js"],`
`5`		`- "random": false`
	`5`	`+ "random": true`
`6`	`6`	`}`