From 9fd707092eeb431b7165b79b329bbd4b58be6942 Mon Sep 17 00:00:00 2001 From: Diamond Lewis Date: Sun, 11 Aug 2024 07:24:50 -0500 Subject: [PATCH] ci: Add test retry logic for flaky tests (#9218) --- spec/Idempotency.spec.js | 4 ++ spec/RegexVulnerabilities.spec.js | 71 +++++++++++----------- spec/helper.js | 3 +- spec/support/CurrentSpecReporter.js | 91 ++++++++++++++++++++++++++--- spec/support/jasmine.json | 2 +- 5 files changed, 129 insertions(+), 42 deletions(-) diff --git a/spec/Idempotency.spec.js b/spec/Idempotency.spec.js index 4031d94704..c8de6a3738 100644 --- 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 index 0064e810f5..b15494af14 100644 --- 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 index 80cf266a27..54937b27a6 100644 --- 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 index 1e7f94362d..0b3aab4fba 100755 --- 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 index 1fbe0c31bf..84d7629c1b 100644 --- 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 }