Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(tests): Include report section for when tests fail due to missing opportunity data (random mode) #647

Merged
merged 18 commits into from
Mar 11, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/openactive-broker-microservice/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -987,6 +987,7 @@ app.post('/test-interface/datasets/:testDatasetIdentifier/opportunities', functi
logError(`Random Bookable Opportunity from seller ${sellerId} for ${criteriaName} within ${bookingFlow} (${opportunityType}) call failed: No matching opportunities found`);
res.status(404).json({
error: `Opportunity of type '${opportunityType}' that has an \`organizer\`/\`provider\` with \`@id\` '${sellerId}', that also matched '${criteriaName}' within '${bookingFlow}', was not found within the feeds.`,
type: 'OpportunityNotFound',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So that this 404 is not confused with other types of 404s when determining if there are missing opportunities

...result,
});
}
Expand Down
1 change: 0 additions & 1 deletion packages/openactive-integration-tests/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

93 changes: 89 additions & 4 deletions packages/openactive-integration-tests/test/helpers/logger.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,27 @@
// TODO fix these issues!
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be a good issue to raise and do, for maintainability

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point @civsiv . I've created an issue to do that, and broadly fix all the ESLint sins, here: #648

/* eslint-disable arrow-parens */
/* eslint-disable prefer-destructuring */
/* eslint-disable no-var */
/* eslint-disable semi */
/* eslint-disable vars-on-top */
/* eslint-disable no-shadow */
/* eslint-disable no-unused-vars */
/* eslint-disable no-param-reassign */
/* eslint-disable arrow-body-style */
/* eslint-disable no-unused-expressions */
/* eslint-disable no-else-return */
/* eslint-disable no-return-assign */
/* eslint-disable comma-dangle */
/* eslint-disable no-trailing-spaces */
/* eslint-disable object-shorthand */
/* eslint-disable getter-return */
/* eslint-disable consistent-return */
/* eslint-disable prefer-const */
/* eslint-disable class-methods-use-this */
/* eslint-disable space-before-function-paren */
/* eslint-disable object-curly-spacing */
/* eslint-disable import/no-useless-path-segments */
/* eslint-disable quotes */
const _ = require("lodash");
const {promises: fs} = require("fs");
const mapping = require('../helpers/mapping');
Expand All @@ -7,12 +31,40 @@ const { getConfigVarOrThrow } = require('./config-utils');
* @typedef {import('chakram').ChakramResponse} ChakramResponse
*/

/**
* @typedef {{
* type: 'OpportunityNotFound';
* opportunityType: string;
* testOpportunityCriteria: string;
* bookingFlow: string;
* sellerId: string;
* }} OpportunityNotFoundEvent An Opportunity was requested from the
* Booking System (possibly via Broker Microservice) but was not found.
* At report generation, this is used to show the user if their Booking
* System has run out of data in random mode.
*
* @typedef {OpportunityNotFoundEvent} FlowStageLogEvent A noteworthy event that
* occurred during a flow stage.
* These events can then be used by the report generator to add detail to the
* flow stage or summarized for the whole test run.
*
* @typedef {{
* request?: Record<string, unknown>;
* response?: {
* validations?: unknown;
* specs?: unknown[];
* };
* events: FlowStageLogEvent[]
* }} FlowStageLog
*/

const OUTPUT_PATH = getConfigVarOrThrow('integrationTests', 'outputPath');
const USE_RANDOM_OPPORTUNITIES = getConfigVarOrThrow('integrationTests', 'useRandomOpportunities');

// abstract class, implement shared methods
class BaseLogger {
constructor () {
/** @type {{[stage: string]: FlowStageLog}} */
this.flow = {};
this.logs = [];
this.timestamp = (new Date()).toString();
Expand Down Expand Up @@ -68,15 +120,29 @@ class BaseLogger {
return log;
}

/**
* Ensure that there is a FlowStageLog stored for the given stage.
* Create one if it doesn't exist.
*
* @param {string} stage
*/
_ensureFlowStage(stage) {
if (!this.flow[stage]) {
this.flow[stage] = {
events: [],
};
}
}

recordRequest (stage, request) {
if (!this.flow[stage]) this.flow[stage] = {};
this._ensureFlowStage(stage);
if (!this.flow[stage].request) this.flow[stage].request = {};

this.flow[stage].request = request;
}

recordResponse (stage, response) {
if (!this.flow[stage]) this.flow[stage] = {};
this._ensureFlowStage(stage);
if (!this.flow[stage].response) this.flow[stage].response = {};

let fields = {
Expand All @@ -103,6 +169,15 @@ class BaseLogger {
Object.assign(this.flow[stage].response, fields);
}

/**
* @param {string} stage
* @param {FlowStageLogEvent} event
*/
recordFlowStageEvent(stage, event) {
this._ensureFlowStage(stage);
this.flow[stage].events.push(event);
}

/**
* @param {string} stage
* @param {{[k: string]: unknown}} request
Expand Down Expand Up @@ -159,7 +234,7 @@ class BaseLogger {
}

recordResponseValidations (stage, data) {
if (!this.flow[stage]) this.flow[stage] = {};
this._ensureFlowStage(stage);
if (!this.flow[stage].response) this.flow[stage].response = {};

this.flow[stage].response.validations = data;
Expand Down Expand Up @@ -341,6 +416,16 @@ class BaseLogger {
get numPassed () {
return this.specStatusCounts.passed;
}

get opportunityTypeName() {
if ('opportunityType' in this && 'bookingFlow' in this) {
return `${this.bookingFlow} >> ${this.opportunityType}`;
}
if ('opportunityType' in this) {
return this.opportunityType;
}
return 'Generic';
}
}

class Logger extends BaseLogger {
Expand Down Expand Up @@ -441,7 +526,7 @@ class ReporterLogger extends BaseLogger {
}

recordTestResult (stage, data) {
if (!this.flow[stage]) this.flow[stage] = {};
this._ensureFlowStage(stage);
if (!this.flow[stage].response) this.flow[stage].response = {};
if (!this.flow[stage].response.specs) this.flow[stage].response.specs = [];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -448,8 +448,9 @@ class RequestHelper {
sellerId,
sellerType,
}) {
const stage = `Local Microservice Test Interface for OrderItem ${orderItemPosition}`;
const respObj = await this.post(
`Local Microservice Test Interface for OrderItem ${orderItemPosition}`,
stage,
`${MICROSERVICE_BASE}/test-interface/datasets/${TEST_DATASET_IDENTIFIER}/opportunities`,
createTestInterfaceOpportunity({
opportunityType,
Expand All @@ -461,6 +462,19 @@ class RequestHelper {
timeout: OPEN_BOOKING_API_REQUEST_TIMEOUT,
},
);
const opportunityNotFound = (
respObj.response.statusCode === 404
&& respObj.body?.type === 'OpportunityNotFound'
);
if (opportunityNotFound) {
this.logger.recordFlowStageEvent(stage, {
type: 'OpportunityNotFound',
opportunityType,
testOpportunityCriteria,
bookingFlow,
sellerId,
});
}
Comment on lines +465 to +477
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where the "opportunity missing" event is saved


return respObj;
}
Expand Down
Loading
Loading