From a703185497b548159155fb606eab048713018163 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Thu, 28 Apr 2022 09:55:20 +0100 Subject: [PATCH] Fixed mockLabs disabling all other flags (#14621) refs TryGhost/Team#1566 - Mocking a labs flag (regardless of enabled/disabled) currently has a side effect of setting any other flag to undefined. - This meant in a test where we set a flag e.g. members-importer where we set multipleProducts, multipleNewsletters is always disabled - This fix preserves the default state of all labs flags that are not mocked so that labs behaves how we expect - Removed usage of GA flags in tests - Removed tests that had GA flags disabled Co-authored-by: Simon Backx --- test/e2e-api/admin/members-importer.test.js | 5 - .../e2e-api/admin/members-newsletters.test.js | 94 ------------------- test/e2e-api/admin/members.test.js | 3 - test/e2e-api/admin/tiers.test.js | 4 - test/e2e-api/members/signin.test.js | 2 - test/e2e-api/members/webhooks.test.js | 43 +++++---- test/unit/frontend/helpers/tiers.test.js | 4 - test/utils/e2e-framework-mock-manager.js | 21 ++++- 8 files changed, 38 insertions(+), 138 deletions(-) diff --git a/test/e2e-api/admin/members-importer.test.js b/test/e2e-api/admin/members-importer.test.js index ee4c118166..b14c32d575 100644 --- a/test/e2e-api/admin/members-importer.test.js +++ b/test/e2e-api/admin/members-importer.test.js @@ -26,11 +26,6 @@ describe('Members Importer API', function () { newsletters = await getNewsletters(); }); - beforeEach(function () { - mockManager.mockLabsEnabled('multipleProducts'); - mockManager.mockLabsEnabled('multipleNewsletters'); - }); - afterEach(function () { mockManager.restore(); }); diff --git a/test/e2e-api/admin/members-newsletters.test.js b/test/e2e-api/admin/members-newsletters.test.js index e425d1ef46..c8d020d493 100644 --- a/test/e2e-api/admin/members-newsletters.test.js +++ b/test/e2e-api/admin/members-newsletters.test.js @@ -1,50 +1,6 @@ const {agentProvider, mockManager, fixtureManager, matchers} = require('../../utils/e2e-framework'); const {anyEtag, anyObjectId, anyUuid, anyISODateTime, anyISODate, anyString, anyArray, anyLocationFor, anyErrorId} = matchers; -const assert = require('assert'); -const nock = require('nock'); -const should = require('should'); -const sinon = require('sinon'); -const testUtils = require('../../utils'); -const Papa = require('papaparse'); - -const models = require('../../../core/server/models'); -const {Product} = require('../../../core/server/models/product'); - -async function assertMemberEvents({eventType, memberId, asserts}) { - const events = await models[eventType].where('member_id', memberId).fetchAll(); - events.map(e => e.toJSON()).should.match(asserts); - assert.equal(events.length, asserts.length, `Only ${asserts.length} ${eventType} should have been added.`); -} - -async function assertSubscription(subscriptionId, asserts) { - // eslint-disable-next-line dot-notation - const subscription = await models['StripeCustomerSubscription'].where('subscription_id', subscriptionId).fetch({require: true}); - - // We use the native toJSON to prevent calling the overriden serialize method - models.Base.Model.prototype.serialize.call(subscription).should.match(asserts); -} - -async function getPaidProduct() { - return await Product.findOne({type: 'paid'}); -} - -const memberMatcherNoIncludes = { - id: anyObjectId, - uuid: anyUuid, - created_at: anyISODateTime, - updated_at: anyISODateTime -}; - -const memberMatcherShallowIncludes = { - id: anyObjectId, - uuid: anyUuid, - created_at: anyISODateTime, - updated_at: anyISODateTime, - subscriptions: anyArray, - labels: anyArray -}; - const memberMatcherShallowIncludesForNewsletters = { id: anyObjectId, uuid: anyUuid, @@ -57,48 +13,6 @@ const memberMatcherShallowIncludesForNewsletters = { let agent; -describe('Members API - No Newsletters', function () { - before(async function () { - agent = await agentProvider.getAdminAPIAgent(); - await fixtureManager.init('members'); - await agent.loginAsOwner(); - }); - - beforeEach(function () { - mockManager.mockLabsDisabled('multipleNewsletters'); - }); - - afterEach(function () { - mockManager.restore(); - }); - - // List Members - - it('Can fetch members who are subscribed', async function () { - await agent - .get('/members/?filter=subscribed:true') - .expectStatus(200) - .matchBodySnapshot({ - members: new Array(6).fill(memberMatcherShallowIncludes) - }) - .matchHeaderSnapshot({ - etag: anyEtag - }); - }); - - it('Can fetch members who are NOT subscribed', async function () { - await agent - .get('/members/?filter=subscribed:false') - .expectStatus(200) - .matchBodySnapshot({ - members: new Array(2).fill(memberMatcherShallowIncludes) - }) - .matchHeaderSnapshot({ - etag: anyEtag - }); - }); -}); - describe('Members API - With Newsletters', function () { before(async function () { agent = await agentProvider.getAdminAPIAgent(); @@ -106,10 +20,6 @@ describe('Members API - With Newsletters', function () { await agent.loginAsOwner(); }); - beforeEach(function () { - mockManager.mockLabsEnabled('multipleNewsletters'); - }); - afterEach(function () { mockManager.restore(); }); @@ -148,10 +58,6 @@ describe('Members API - With Newsletters - compat mode', function () { await agent.loginAsOwner(); }); - beforeEach(function () { - mockManager.mockLabsEnabled('multipleNewsletters'); - }); - afterEach(function () { mockManager.restore(); }); diff --git a/test/e2e-api/admin/members.test.js b/test/e2e-api/admin/members.test.js index 42b0f62f36..9168dd0d76 100644 --- a/test/e2e-api/admin/members.test.js +++ b/test/e2e-api/admin/members.test.js @@ -132,9 +132,6 @@ describe('Members API', function () { }); beforeEach(function () { - mockManager.mockLabsEnabled('multipleProducts'); - mockManager.mockLabsEnabled('multipleNewsletters'); - mockManager.mockLabsEnabled('membersActivityFeed'); mockManager.mockStripe(); mockManager.mockMail(); }); diff --git a/test/e2e-api/admin/tiers.test.js b/test/e2e-api/admin/tiers.test.js index 2fa404d660..009f49da88 100644 --- a/test/e2e-api/admin/tiers.test.js +++ b/test/e2e-api/admin/tiers.test.js @@ -15,10 +15,6 @@ describe('Tiers API', function () { await agent.loginAsOwner(); }); - beforeEach(function () { - mockManager.mockLabsEnabled('multipleProducts'); - }); - afterEach(function () { mockManager.restore(); }); diff --git a/test/e2e-api/members/signin.test.js b/test/e2e-api/members/signin.test.js index 0d366801f7..701538f43c 100644 --- a/test/e2e-api/members/signin.test.js +++ b/test/e2e-api/members/signin.test.js @@ -13,8 +13,6 @@ describe('Members Signin', function () { }); beforeEach(function () { - mockManager.mockLabsEnabled('multipleProducts'); - mockManager.mockLabsEnabled('tierWelcomePages'); mockManager.mockStripe(); }); diff --git a/test/e2e-api/members/webhooks.test.js b/test/e2e-api/members/webhooks.test.js index 16db08a965..384d6c52e2 100644 --- a/test/e2e-api/members/webhooks.test.js +++ b/test/e2e-api/members/webhooks.test.js @@ -44,8 +44,7 @@ describe('Members API', function () { }); beforeEach(function () { - mockManager.mockLabsEnabled('multipleProducts'); - mockManager.mockLabsEnabled('multipleNewsletters'); + mockManager.mockLabsDisabled('dashboardV5'); mockManager.mockMail(); mockManager.mockStripe(); }); @@ -1105,7 +1104,7 @@ describe('Members API', function () { set(coupon, { id: couponId }); - + const {body} = await adminAgent .post(`offers/`) .body({offers: [newOffer]}) @@ -1114,7 +1113,7 @@ describe('Members API', function () { }); /** - * Helper for repetitive tests. It tests the MRR and MRR delta given a discount + a price + * Helper for repetitive tests. It tests the MRR and MRR delta given a discount + a price */ async function testDiscount({discount, interval, unit_amount, assert_mrr, offer_id}) { const customer_id = createStripeID('cust'); @@ -1172,7 +1171,7 @@ describe('Members API', function () { } } }); - + let webhookSignature = stripe.webhooks.generateTestHeaderString({ payload: webhookPayload, secret: process.env.WEBHOOK_SECRET @@ -1222,7 +1221,7 @@ describe('Members API', function () { // Now cancel, and check if the discount is also applied for the cancellation set(subscription, { - ...subscription, + ...subscription, status: 'canceled' }); @@ -1320,7 +1319,7 @@ describe('Members API', function () { promotion_code: null, start: beforeNow / 1000, subscription: null - }; + }; await testDiscount({ discount, unit_amount: 500, @@ -1330,7 +1329,7 @@ describe('Members API', function () { }); }); - it('Correctly includes yearly forever percentage discounts in MRR', async function () { + it('Correctly includes yearly forever percentage discounts in MRR', async function () { const discount = { id: 'di_1Knkn7HUEDadPGIBPOQgmzIX', object: 'discount', @@ -1358,7 +1357,7 @@ describe('Members API', function () { promotion_code: null, start: beforeNow / 1000, subscription: null - }; + }; await testDiscount({ discount, unit_amount: 1200, @@ -1368,7 +1367,7 @@ describe('Members API', function () { }); }); - it('Correctly includes monthly forever amount off discounts in MRR', async function () { + it('Correctly includes monthly forever amount off discounts in MRR', async function () { const discount = { id: 'di_1Knkn7HUEDadPGIBPOQgmzIX', object: 'discount', @@ -1396,7 +1395,7 @@ describe('Members API', function () { promotion_code: null, start: beforeNow / 1000, subscription: null - }; + }; await testDiscount({ discount, unit_amount: 500, @@ -1406,7 +1405,7 @@ describe('Members API', function () { }); }); - it('Correctly includes yearly forever amount off discounts in MRR', async function () { + it('Correctly includes yearly forever amount off discounts in MRR', async function () { const discount = { id: 'di_1Knkn7HUEDadPGIBPOQgmzIX', object: 'discount', @@ -1434,7 +1433,7 @@ describe('Members API', function () { promotion_code: null, start: beforeNow / 1000, subscription: null - }; + }; await testDiscount({ discount, unit_amount: 1200, @@ -1444,7 +1443,7 @@ describe('Members API', function () { }); }); - it('Does not include repeating discounts in MRR', async function () { + it('Does not include repeating discounts in MRR', async function () { const discount = { id: 'di_1Knkn7HUEDadPGIBPOQgmzIX', object: 'discount', @@ -1472,7 +1471,7 @@ describe('Members API', function () { promotion_code: null, start: beforeNow / 1000, subscription: null - }; + }; await testDiscount({ discount, unit_amount: 500, @@ -1572,7 +1571,7 @@ describe('Members API', function () { } } }); - + let webhookSignature = stripe.webhooks.generateTestHeaderString({ payload: webhookPayload, secret: process.env.WEBHOOK_SECRET @@ -1619,7 +1618,7 @@ describe('Members API', function () { // Now add the discount set(subscription, { - ...subscription, + ...subscription, discount }); @@ -1768,7 +1767,7 @@ describe('Members API', function () { } } }); - + let webhookSignature = stripe.webhooks.generateTestHeaderString({ payload: webhookPayload, secret: process.env.WEBHOOK_SECRET @@ -1816,7 +1815,7 @@ describe('Members API', function () { }); describe('Without the dashboardV5 flag', function () { - it('Does not include forever percentage discounts in MRR', async function () { + it('Does not include forever percentage discounts in MRR', async function () { const discount = { id: 'di_1Knkn7HUEDadPGIBPOQgmzIX', object: 'discount', @@ -1844,7 +1843,7 @@ describe('Members API', function () { promotion_code: null, start: beforeNow / 1000, subscription: null - }; + }; await testDiscount({ discount, unit_amount: 500, @@ -1854,7 +1853,7 @@ describe('Members API', function () { }); }); - it('Does not include forever amount off discounts in MRR', async function () { + it('Does not include forever amount off discounts in MRR', async function () { const discount = { id: 'di_1Knkn7HUEDadPGIBPOQgmzIX', object: 'discount', @@ -1882,7 +1881,7 @@ describe('Members API', function () { promotion_code: null, start: beforeNow / 1000, subscription: null - }; + }; await testDiscount({ discount, unit_amount: 500, diff --git a/test/unit/frontend/helpers/tiers.test.js b/test/unit/frontend/helpers/tiers.test.js index 6fac4c6087..bb337e4f3c 100644 --- a/test/unit/frontend/helpers/tiers.test.js +++ b/test/unit/frontend/helpers/tiers.test.js @@ -3,10 +3,6 @@ const tiersHelper = require('../../../../core/frontend/helpers/tiers'); const {mockManager} = require('../../../utils/e2e-framework'); describe('{{tiers}} helper', function () { - beforeEach(function () { - mockManager.mockLabsEnabled('multipleProducts'); - }); - it('can return string with tiers', function () { const tiers = [ {name: 'tier 1'}, diff --git a/test/utils/e2e-framework-mock-manager.js b/test/utils/e2e-framework-mock-manager.js index 373a2b7902..b6cd0a9af6 100644 --- a/test/utils/e2e-framework-mock-manager.js +++ b/test/utils/e2e-framework-mock-manager.js @@ -14,6 +14,9 @@ const mailService = require('../../core/server/services/mail/index'); const labs = require('../../core/shared/labs'); const events = require('../../core/server/lib/common/events'); +let fakedLabsFlags = {}; +const originalLabsIsSet = labs.isSet; + /** * Stripe Mocks */ @@ -93,6 +96,15 @@ const emittedEvent = (name) => { /** * Labs Mocks */ + +const fakeLabsIsSet = (flag) => { + if (fakedLabsFlags.hasOwnProperty(flag)) { + return fakedLabsFlags[flag]; + } + + return originalLabsIsSet(flag); +}; + const mockLabsEnabled = (flag, alpha = true) => { // We assume we should enable alpha experiments unless explicitly told not to! if (!alpha) { @@ -100,10 +112,10 @@ const mockLabsEnabled = (flag, alpha = true) => { } if (!mocks.labs) { - mocks.labs = sinon.stub(labs, 'isSet'); + mocks.labs = sinon.stub(labs, 'isSet').callsFake(fakeLabsIsSet); } - mocks.labs.withArgs(flag).returns(true); + fakedLabsFlags[flag] = true; }; const mockLabsDisabled = (flag, alpha = true) => { @@ -113,16 +125,17 @@ const mockLabsDisabled = (flag, alpha = true) => { } if (!mocks.labs) { - mocks.labs = sinon.stub(labs, 'isSet'); + mocks.labs = sinon.stub(labs, 'isSet').callsFake(fakeLabsIsSet); } - mocks.labs.withArgs(flag).returns(false); + fakedLabsFlags[flag] = false; }; const restore = () => { configUtils.restore(); sinon.restore(); mocks = {}; + fakedLabsFlags = {}; emailCount = 0; nock.cleanAll(); nock.enableNetConnect();