Skip to content

Commit

Permalink
REST: Fix flaky mocha tests caused by AuthTest.js
Browse files Browse the repository at this point in the history
Occasionally, mocha tests in the `mediawiki-quibble-apitests-vendor-php74` CI job fail with the following error:
```
{
  code: 'permission-denied',
  message: 'Access to resource is denied',
  context: { denial_reason: 'resource-protected' }
}
```

I suspect this is due to tests in `AuthTest.js` protecting an item/property that is also used in other tests. `AuthTest.js` uses the `describeWithTestData()` helper function which in turn uses the `getItemId()` helper function which enable the reuse of a single Item across tests.

Adding a `createNewItem` param to `describeWithTestData()` allows `AuthTest.js` to request that a new Item is created for its tests.

Change-Id: I0ba157a6bffdf8facc9a99905ae9287e71f8d4e1
  • Loading branch information
outdooracorn committed Feb 25, 2025
1 parent 2012bdd commit d508850
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 5 deletions.
3 changes: 1 addition & 2 deletions repo/domains/crud/tests/mocha/api-testing/AuthTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ describeWithTestData( 'Auth', ( itemRequestInputs, propertyRequestInputs, descri
} );

describe( 'Authorization', () => {

describeEachRouteWithReset(
[
...editRequests,
Expand Down Expand Up @@ -159,4 +158,4 @@ describeWithTestData( 'Auth', ( itemRequestInputs, propertyRequestInputs, descri
expect( response ).to.have.status( 403 );
} );
} );
} );
}, true ); // create a new Item for the Auth tests
8 changes: 5 additions & 3 deletions repo/domains/crud/tests/mocha/helpers/describeWithTestData.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const {
} = require( './entityHelper' );
const { getAllowedBadges } = require( './getAllowedBadges' );
const {
newCreateItemRequestBuilder,
newPatchItemRequestBuilder,
newPatchPropertyRequestBuilder
} = require( './RequestBuilderFactory' );
Expand All @@ -31,9 +32,10 @@ const { expect } = require( '../../../../../rest-api/tests/mocha/helpers/chaiHel
*
* @param {string} testName
* @param {Function} runAllTests
* @param {boolean} createNewItem
* @return {void}
*/
function describeWithTestData( testName, runAllTests ) {
function describeWithTestData( testName, runAllTests, createNewItem = false ) {
const itemRequestInputs = {};
const propertyRequestInputs = {};
const originalLinkedArticle = utils.title( 'Original-article-linked-to-test-item' );
Expand Down Expand Up @@ -86,7 +88,8 @@ function describeWithTestData( testName, runAllTests ) {
await createWikiPage( newLinkedArticle, 'sitelink test' );
const statementPropertyId = await getStringPropertyId();

const itemId = await getItemId();
const itemId = createNewItem ?
( await newCreateItemRequestBuilder( {} ).makeRequest() ).body.id : await getItemId();
const item = await resetEntityTestData( itemId, statementPropertyId, originalLinkedArticle );
itemRequestInputs.mainTestSubject = itemId;
itemRequestInputs.itemId = itemId;
Expand All @@ -101,7 +104,6 @@ function describeWithTestData( testName, runAllTests ) {
propertyRequestInputs.propertyId = propertyId;
propertyRequestInputs.statementId = property.statements[ statementPropertyId ][ 0 ].id;
propertyRequestInputs.statementPropertyId = statementPropertyId;

} );

runAllTests( itemRequestInputs, propertyRequestInputs, describeEachRouteWithReset );
Expand Down

0 comments on commit d508850

Please sign in to comment.