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 <simon@ghost.org>
This commit is contained in:
Hannah Wolfe 2022-04-28 09:55:20 +01:00 committed by GitHub
parent efdc42c257
commit a703185497
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 38 additions and 138 deletions

View File

@ -26,11 +26,6 @@ describe('Members Importer API', function () {
newsletters = await getNewsletters();
});
beforeEach(function () {
mockManager.mockLabsEnabled('multipleProducts');
mockManager.mockLabsEnabled('multipleNewsletters');
});
afterEach(function () {
mockManager.restore();
});

View File

@ -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();
});

View File

@ -132,9 +132,6 @@ describe('Members API', function () {
});
beforeEach(function () {
mockManager.mockLabsEnabled('multipleProducts');
mockManager.mockLabsEnabled('multipleNewsletters');
mockManager.mockLabsEnabled('membersActivityFeed');
mockManager.mockStripe();
mockManager.mockMail();
});

View File

@ -15,10 +15,6 @@ describe('Tiers API', function () {
await agent.loginAsOwner();
});
beforeEach(function () {
mockManager.mockLabsEnabled('multipleProducts');
});
afterEach(function () {
mockManager.restore();
});

View File

@ -13,8 +13,6 @@ describe('Members Signin', function () {
});
beforeEach(function () {
mockManager.mockLabsEnabled('multipleProducts');
mockManager.mockLabsEnabled('tierWelcomePages');
mockManager.mockStripe();
});

View File

@ -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,

View File

@ -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'},

View File

@ -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();