From e5c0ae410a00054a5adc2b73330cf3b4f6f0ae94 Mon Sep 17 00:00:00 2001 From: Pagan Gazzard Date: Tue, 7 Jul 2020 13:37:06 +0100 Subject: [PATCH] Remove `this.skip` usage as a faster workaround to afterEach skipping See: https://github.com/mochajs/mocha/issues/3740 Change-type: patch --- .../models/application.spec.coffee | 16 ++--- tests/integration/models/billing.spec.ts | 11 ++-- tests/integration/models/tags.ts | 65 ++++++++----------- tests/integration/setup.coffee | 24 ++----- tests/util_ts.ts | 13 +--- 5 files changed, 44 insertions(+), 85 deletions(-) diff --git a/tests/integration/models/application.spec.coffee b/tests/integration/models/application.spec.coffee index 8ff966678..f063256aa 100644 --- a/tests/integration/models/application.spec.coffee +++ b/tests/integration/models/application.spec.coffee @@ -921,13 +921,11 @@ describe 'Application Model', -> before -> balena.auth.logout() - describe 'arbitrary pinejs queries', -> + $it = if publicApp then it else it.skip - it 'should be able to retrieve the available public apps', -> - if !publicApp - this.skip() - return + describe 'arbitrary pinejs queries', -> + $it 'should be able to retrieve the available public apps', -> balena.pine.get resource: 'application' options: @@ -946,18 +944,12 @@ describe 'Application Model', -> describe 'balena.models.application.get()', -> - [ 'id' 'app_name' 'slug' ].forEach (prop) -> - - it "should be able to get a public application by #{prop}", -> - if !publicApp - this.skip() - return - + $it "should be able to get a public application by #{prop}", -> balena.models.application.get(publicApp[prop]) .then (app) -> m.chai.expect(app).to.have.property('id').that.is.a('number') diff --git a/tests/integration/models/billing.spec.ts b/tests/integration/models/billing.spec.ts index 886c2c52b..cced5c2ad 100644 --- a/tests/integration/models/billing.spec.ts +++ b/tests/integration/models/billing.spec.ts @@ -148,13 +148,10 @@ describe('Billing Model', function () { const givenABillingAccountIt = ( description: string, testFn: (...args: any[]) => any, - ) => - it(description, function () { - if (!hasActiveBillingAccount) { - return this.skip(); - } - return testFn.apply(this, arguments); - }); + ) => { + const $it = hasActiveBillingAccount ? it : it.skip; + $it(description, testFn); + }; before(() => loginPaidUser() diff --git a/tests/integration/models/tags.ts b/tests/integration/models/tags.ts index 7e67e6487..e4578360f 100644 --- a/tests/integration/models/tags.ts +++ b/tests/integration/models/tags.ts @@ -136,30 +136,25 @@ exports.itShouldSetGetAndRemoveTags = function (opts: Options) { ['id', ...uniquePropertyNames].forEach((param) => describe(`given a ${resourceName} ${param}`, function () { - it(`should be rejected if the ${resourceName} id does not exist`, function () { - if (!param) { - return this.skip(); - } - const resourceUniqueKey = param === 'id' ? 999999 : '123456789'; - const promise = model.set(resourceUniqueKey, 'EDITOR', 'vim'); - return expect(promise).to.be.rejectedWith( - `${_.startCase(resourceName)} not found: ${resourceUniqueKey}`, - ); - }); + const $it = param ? it : it.skip; + $it( + `should be rejected if the ${resourceName} id does not exist`, + function () { + const resourceUniqueKey = param === 'id' ? 999999 : '123456789'; + const promise = model.set(resourceUniqueKey, 'EDITOR', 'vim'); + return expect(promise).to.be.rejectedWith( + `${_.startCase(resourceName)} not found: ${resourceUniqueKey}`, + ); + }, + ); - it('should initially have no tags', function () { - if (!param) { - return this.skip(); - } + $it('should initially have no tags', function () { return getAllByResource(this.resource[param]).then((tags) => expect(tags).to.have.length(0), ); }); - it('...should be able to create a tag', function () { - if (!param) { - return this.skip(); - } + $it('...should be able to create a tag', function () { const promise = model.set( this.resource[param], `EDITOR_BY_${resourceName}_${param}`, @@ -168,23 +163,20 @@ exports.itShouldSetGetAndRemoveTags = function (opts: Options) { return expect(promise).to.not.be.rejected; }); - it('...should be able to retrieve all tags, including the one created', function () { - if (!param) { - return this.skip(); - } - return getAllByResource(this.resource[param]).then(function (tags) { - expect(tags).to.have.length(1); - const tag = tags[0]; - expect(tag).to.be.an('object'); - expect(tag.tag_key).to.equal(`EDITOR_BY_${resourceName}_${param}`); - expect(tag.value).to.equal('vim'); - }); - }); + $it( + '...should be able to retrieve all tags, including the one created', + function () { + return getAllByResource(this.resource[param]).then(function (tags) { + expect(tags).to.have.length(1); + const tag = tags[0]; + expect(tag).to.be.an('object'); + expect(tag.tag_key).to.equal(`EDITOR_BY_${resourceName}_${param}`); + expect(tag.value).to.equal('vim'); + }); + }, + ); - it('...should be able to update a tag', function () { - if (!param) { - return this.skip(); - } + $it('...should be able to update a tag', function () { return model .set( this.resource[param], @@ -203,10 +195,7 @@ exports.itShouldSetGetAndRemoveTags = function (opts: Options) { }); }); - it('...should be able to remove a tag', function () { - if (!param) { - return this.skip(); - } + $it('...should be able to remove a tag', function () { return model .remove(this.resource[param], `EDITOR_BY_${resourceName}_${param}`) .then(() => { diff --git a/tests/integration/setup.coffee b/tests/integration/setup.coffee index 81a44cc4e..4fda2219f 100644 --- a/tests/integration/setup.coffee +++ b/tests/integration/setup.coffee @@ -146,16 +146,11 @@ exports.givenAnApplication = (beforeFn) -> exports.givenInitialOrganization(beforeFn) beforeFn -> - # calling this.skip() doesn't trigger afterEach, - # so we need to reset in here as well - # See: https://github.com/mochajs/mocha/issues/3740 - resetApplications() - .then => - balena.models.application.create - name: 'FooBar' - applicationType: 'microservices-starter' - deviceType: 'raspberry-pi' - organization: @initialOrg.id + balena.models.application.create + name: 'FooBar' + applicationType: 'microservices-starter' + deviceType: 'raspberry-pi' + organization: @initialOrg.id .then (@application) => chai.expect(@application.is_for__device_type).to.be.an('object') .that.has.property('__id').that.is.a('number') @@ -180,13 +175,8 @@ resetDevices = -> exports.givenADevice = (beforeFn, extraDeviceProps) -> beforeFn -> - # calling this.skip() doesn't trigger afterEach, - # so we need to reset in here as well - # See: https://github.com/mochajs/mocha/issues/3740 - resetDevices() - .then => - uuid = balena.models.device.generateUniqueKey() - balena.models.device.register(@application.app_name, uuid) + uuid = balena.models.device.generateUniqueKey() + balena.models.device.register(@application.app_name, uuid) .tap (deviceInfo) => if !@currentRelease || !@currentRelease.commit return diff --git a/tests/util_ts.ts b/tests/util_ts.ts index 708a1883d..9d3b3082a 100644 --- a/tests/util_ts.ts +++ b/tests/util_ts.ts @@ -9,7 +9,7 @@ export const describeExpandAssertions = async ( describe(`expanding from ${params.resource}`, function () { Object.keys(params.options.$expand).forEach((key) => { describe(`to ${key}`, function () { - it('should succeed', async function () { + it('should succeed and include the expanded property', async function () { const [result] = await balena.pine.get({ ...params, options: { @@ -19,16 +19,7 @@ export const describeExpandAssertions = async ( }, }, }); - this.result = result; - }); - - it('should include the expanded property', function () { - if (!this.result) { - this.skip(); - return; - } - - expect(this.result).to.have.property(key).that.is.an('array'); + expect(result).to.have.property(key).that.is.an('array'); }); }); });