Skip to content

Commit 9ff98ad

Browse files
authored
Break test order dependencies in security-rules integration tests. (#673)
Previously, this would work: yarn test:integration --updateRules --grep 'admin.securityRules' but this would not: yarn test:integration --updateRules --grep 'admin.securityRules.getRuleset\(\).rejects with not-found' because that test relied on side effects from earier tests, making this test suite difficult to debug.
1 parent 6ea4a9f commit 9ff98ad

File tree

1 file changed

+47
-31
lines changed

1 file changed

+47
-31
lines changed

test/integration/security-rules.spec.ts

Lines changed: 47 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@ const RULESET_NAME_PATTERN = /[0-9a-zA-Z-]+/;
4545

4646
describe('admin.securityRules', () => {
4747

48-
let testRuleset: admin.securityRules.Ruleset = null;
4948
const rulesetsToDelete: string[] = [];
5049

5150
function scheduleForDelete(ruleset: admin.securityRules.Ruleset) {
@@ -65,7 +64,17 @@ describe('admin.securityRules', () => {
6564
return Promise.all(promises);
6665
}
6766

68-
after(() => {
67+
function createTemporaryRuleset(): Promise<admin.securityRules.Ruleset> {
68+
const name = 'firestore.rules';
69+
const rulesFile = admin.securityRules().createRulesFileFromSource(name, SAMPLE_FIRESTORE_RULES);
70+
return admin.securityRules().createRuleset(rulesFile)
71+
.then((ruleset) => {
72+
scheduleForDelete(ruleset);
73+
return ruleset;
74+
});
75+
}
76+
77+
afterEach(() => {
6978
return deleteTempRulesets();
7079
});
7180

@@ -91,7 +100,6 @@ describe('admin.securityRules', () => {
91100
RULES_FILE_NAME, SAMPLE_FIRESTORE_RULES);
92101
return admin.securityRules().createRuleset(rulesFile)
93102
.then((ruleset) => {
94-
testRuleset = ruleset;
95103
scheduleForDelete(ruleset);
96104
verifyFirestoreRuleset(ruleset);
97105
});
@@ -107,38 +115,41 @@ describe('admin.securityRules', () => {
107115

108116
describe('getRuleset()', () => {
109117
it('rejects with not-found when the Ruleset does not exist', () => {
110-
const name = 'e1212' + testRuleset.name.substring(5);
111-
return admin.securityRules().getRuleset(name)
118+
const nonExistingName = '00000000-1111-2222-3333-444444444444';
119+
return admin.securityRules().getRuleset(nonExistingName)
112120
.should.eventually.be.rejected.and.have.property('code', 'security-rules/not-found');
113121
});
114122

115123
it('rejects with invalid-argument when the Ruleset name is invalid', () => {
116-
return admin.securityRules().getRuleset('invalid')
124+
return admin.securityRules().getRuleset('invalid uuid')
117125
.should.eventually.be.rejected.and.have.property('code', 'security-rules/invalid-argument');
118126
});
119127

120128
it('resolves with existing Ruleset', () => {
121-
return admin.securityRules().getRuleset(testRuleset.name)
122-
.then((ruleset) => {
123-
verifyFirestoreRuleset(ruleset);
124-
});
129+
return createTemporaryRuleset()
130+
.then((expectedRuleset) =>
131+
admin.securityRules().getRuleset(expectedRuleset.name)
132+
.then((actualRuleset) => {
133+
expect(actualRuleset).to.deep.equal(expectedRuleset);
134+
}),
135+
);
125136
});
126137
});
127138

128139
describe('Cloud Firestore', () => {
129140
let oldRuleset: admin.securityRules.Ruleset = null;
130141
let newRuleset: admin.securityRules.Ruleset = null;
131142

132-
function revertFirestoreRuleset(): Promise<void> {
143+
function revertFirestoreRulesetIfModified(): Promise<void> {
133144
if (!newRuleset) {
134145
return Promise.resolve();
135146
}
136147

137148
return admin.securityRules().releaseFirestoreRuleset(oldRuleset);
138149
}
139150

140-
after(() => {
141-
return revertFirestoreRuleset();
151+
afterEach(() => {
152+
return revertFirestoreRulesetIfModified();
142153
});
143154

144155
it('getFirestoreRuleset() returns the Ruleset currently in effect', () => {
@@ -177,16 +188,16 @@ describe('admin.securityRules', () => {
177188
let oldRuleset: admin.securityRules.Ruleset = null;
178189
let newRuleset: admin.securityRules.Ruleset = null;
179190

180-
function revertStorageRuleset(): Promise<void> {
191+
function revertStorageRulesetIfModified(): Promise<void> {
181192
if (!newRuleset) {
182193
return Promise.resolve();
183194
}
184195

185196
return admin.securityRules().releaseStorageRuleset(oldRuleset);
186197
}
187198

188-
after(() => {
189-
return revertStorageRuleset();
199+
afterEach(() => {
200+
return revertStorageRulesetIfModified();
190201
});
191202

192203
it('getStorageRuleset() returns the currently applied Storage rules', () => {
@@ -240,9 +251,13 @@ describe('admin.securityRules', () => {
240251
});
241252
}
242253

243-
return listAllRulesets()
244-
.then((rulesets) => {
245-
expect(rulesets.some((rs) => rs.name === testRuleset.name)).to.be.true;
254+
return Promise.all([createTemporaryRuleset(), createTemporaryRuleset()])
255+
.then((expectedRulesets) => {
256+
return listAllRulesets().then((actualRulesets) => {
257+
expectedRulesets.forEach((expectedRuleset) => {
258+
expect(actualRulesets.map((r) => r.name)).to.deep.include(expectedRuleset.name);
259+
});
260+
});
246261
});
247262
});
248263

@@ -257,26 +272,27 @@ describe('admin.securityRules', () => {
257272

258273
describe('deleteRuleset()', () => {
259274
it('rejects with not-found when the Ruleset does not exist', () => {
260-
const name = 'e1212' + testRuleset.name.substring(5);
261-
return admin.securityRules().deleteRuleset(name)
275+
const nonExistingName = '00000000-1111-2222-3333-444444444444';
276+
return admin.securityRules().deleteRuleset(nonExistingName)
262277
.should.eventually.be.rejected.and.have.property('code', 'security-rules/not-found');
263278
});
264279

265280
it('rejects with invalid-argument when the Ruleset name is invalid', () => {
266-
return admin.securityRules().deleteRuleset('invalid')
281+
return admin.securityRules().deleteRuleset('invalid uuid')
267282
.should.eventually.be.rejected.and.have.property('code', 'security-rules/invalid-argument');
268283
});
269284

270285
it('deletes existing Ruleset', () => {
271-
return admin.securityRules().deleteRuleset(testRuleset.name)
272-
.then(() => {
273-
return admin.securityRules().getRuleset(testRuleset.name)
274-
.should.eventually.be.rejected.and.have.property('code', 'security-rules/not-found');
275-
})
276-
.then(() => {
277-
unscheduleForDelete(testRuleset); // Already deleted.
278-
testRuleset = null;
279-
});
286+
return createTemporaryRuleset().then((ruleset) => {
287+
return admin.securityRules().deleteRuleset(ruleset.name)
288+
.then(() => {
289+
return admin.securityRules().getRuleset(ruleset.name)
290+
.should.eventually.be.rejected.and.have.property('code', 'security-rules/not-found');
291+
})
292+
.then(() => {
293+
unscheduleForDelete(ruleset); // Already deleted.
294+
});
295+
});
280296
});
281297
});
282298

0 commit comments

Comments
 (0)