diff --git a/packages/openactive-broker-microservice/src/core.js b/packages/openactive-broker-microservice/src/core.js index 99b42a1231..a4997fbb37 100644 --- a/packages/openactive-broker-microservice/src/core.js +++ b/packages/openactive-broker-microservice/src/core.js @@ -277,6 +277,7 @@ function getRandomOpportunityRoute(req, res) { 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', ...result, }); } diff --git a/packages/openactive-integration-tests/test/helpers/logger.js b/packages/openactive-integration-tests/test/helpers/logger.js index 3c48f412b5..012d56d1a6 100644 --- a/packages/openactive-integration-tests/test/helpers/logger.js +++ b/packages/openactive-integration-tests/test/helpers/logger.js @@ -1,3 +1,27 @@ +// TODO fix these issues! +/* 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'); @@ -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; + * 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(); @@ -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 = { @@ -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 @@ -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; @@ -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 { @@ -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 = []; diff --git a/packages/openactive-integration-tests/test/helpers/request-helper.js b/packages/openactive-integration-tests/test/helpers/request-helper.js index 668ca83ae4..6f0006fe4a 100644 --- a/packages/openactive-integration-tests/test/helpers/request-helper.js +++ b/packages/openactive-integration-tests/test/helpers/request-helper.js @@ -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, @@ -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, + }); + } return respObj; } diff --git a/packages/openactive-integration-tests/test/report-generator.js b/packages/openactive-integration-tests/test/report-generator.js index 4cd68436b5..ec7b4fee66 100644 --- a/packages/openactive-integration-tests/test/report-generator.js +++ b/packages/openactive-integration-tests/test/report-generator.js @@ -1,3 +1,4 @@ +const util = require('util'); const chalk = require("chalk"); const Handlebars = require("handlebars"); const fs = require("fs").promises; @@ -7,6 +8,17 @@ const _ = require("lodash"); const { getConfigVarOrThrow } = require('./helpers/config-utils'); const showdown = require('showdown'); +/** + * @typedef {{ + * [testOpportunityCriteria: string]: { + * sellerIds: string[]; + * opportunityTypes: string[]; + * bookingFlows: string[]; + * numTestsFailing: number; + * } + * }} MissingOpportunityDataSummary + */ + const USE_RANDOM_OPPORTUNITIES = getConfigVarOrThrow('integrationTests', 'useRandomOpportunities'); const OUTPUT_PATH = getConfigVarOrThrow('integrationTests', 'outputPath'); const ENABLE_HEADER_LOGGING = getConfigVarOrThrow('integrationTests', 'requestHeaderLogging'); @@ -105,6 +117,7 @@ class BaseReportGenerator { }, "logsFor": (suite, type, options) => { let first = true; + // @ts-expect-error this.logger is only defined in ReportGenerator let logs = this.logger.logsFor(suite, type); let ret = ""; for (let [i, value] of logs.entries()) { @@ -128,6 +141,7 @@ class BaseReportGenerator { return ret; }, "statusFor": (suite) => { + // @ts-expect-error this.logger is only defined in ReportGenerator let status = this.logger.statusFor(suite); return status; }, @@ -145,6 +159,9 @@ class BaseReportGenerator { return {}; } + /** + * @returns {string} + */ get reportHtmlPath () { throw "Not Implemented"; } @@ -180,7 +197,6 @@ class BaseReportGenerator { converter.setOption('openLinksInNewWindow', true); const html = converter.makeHtml(data); - await fs.writeFile(this.reportHtmlPath, html); } @@ -260,7 +276,7 @@ class SummaryReportGenerator extends BaseReportGenerator { numFailed })) })) - ) + ), }; } @@ -274,7 +290,7 @@ class SummaryReportGenerator extends BaseReportGenerator { return "summary"; } - get templateData () { + get templateData() { return this; } @@ -302,9 +318,24 @@ class SummaryReportGenerator extends BaseReportGenerator { return (this.datasetJson.bookingService && this.datasetJson.bookingService.name) || (this.datasetJson.publisher && this.datasetJson.publisher.name); } + + get missingOpportunityDataSummary() { + if (this._missingOpportunityDataSummary !== undefined) { + return this._missingOpportunityDataSummary; + } + const { missingOpportunityDataSummary } = this.loggers; + if (_.isEmpty(missingOpportunityDataSummary)) { + return null; + } + this._missingOpportunityDataSummary = missingOpportunityDataSummary; + return this._missingOpportunityDataSummary; + } } class LoggerGroup { + /** + * @param {import('./helpers/logger').BaseLoggerType[]} loggers + */ constructor (reporter, loggers) { this.reporter = reporter; this.loggers = loggers; @@ -315,6 +346,7 @@ class LoggerGroup { return this._opportunityTypeGroups = _ .chain(this.loggers) + // @ts-expect-error logger.opportunityType and logger.bookingFlow are only defined when a logger is loaded from an output JSON .groupBy(logger => logger.opportunityType ? `${logger.bookingFlow} >> ${logger.opportunityType}` : "Generic") .mapValues(group => new LoggerGroup(this.reporter, group)) .value(); @@ -332,7 +364,7 @@ class LoggerGroup { } get opportunityTypeName() { - return this.loggers[0].opportunityType ? (`${this.loggers[0].bookingFlow} >> ${this.loggers[0].opportunityType}`) : 'Generic'; + return this.loggers[0].opportunityTypeName; } get featureName () { @@ -366,7 +398,7 @@ class LoggerGroup { acc[key] = (acc[key] || 0) + value; } return acc; - }, {}); + }, /** @type {Record} */({})); } get validationStatusCounts () { @@ -377,7 +409,7 @@ class LoggerGroup { acc[key] = (acc[key] || 0) + value; } return acc; - }, {}); + }, /** @type {Record} */({})); } get overallStatus () { @@ -392,6 +424,83 @@ class LoggerGroup { else return "passed"; } + /** + * Get stats about which tests have OpportunityNotFound events and thus + * failed due to missing required opportunity data. + */ + get missingOpportunityDataStats() { + if (this._missingOpportunityDataStats) { return this._missingOpportunityDataStats; } + const allAugmentedEvents = this.loggers.flatMap((logger) => { + const testConfig = _.pick(logger, [ + 'opportunityTypeName', + + 'featureName', + 'implementedDisplayLabel', + 'suiteName', + 'htmlLocalPath', + ]); + const flowStageLogs = Object.values(logger.flow); + return flowStageLogs.flatMap(flowStageLog => ( + flowStageLog.events + .filter(event => event.type === 'OpportunityNotFound') + // Associate each of these events with a specific test + .map(event => ({ ..._.omit(event, ['type']), testConfig })) + )); + }); + const uniqueAugmentedEvents = _.uniqWith(allAugmentedEvents, _.isEqual); + this._missingOpportunityDataStats = uniqueAugmentedEvents; + return this._missingOpportunityDataStats; + } + + /** + * Groups the results from `missingOpportunityDataStats` into something + * that can be shown in the summary report. + */ + get missingOpportunityDataSummary() { + if (this._missingOpportunityDataSummary) { return this._missingOpportunityDataSummary; } + const stats = this.missingOpportunityDataStats; + // So that criteria are listed alphabetically in the summary report + const statsSortedByCriteria = _.sortBy(stats, [ + 'testOpportunityCriteria', + ]); + this._missingOpportunityDataSummary = statsSortedByCriteria.reduce((acc, event) => { + if (!acc[event.testOpportunityCriteria]) { + acc[event.testOpportunityCriteria] = { + sellerIds: [], + opportunityTypes: [], + bookingFlows: [], + numTestsFailing: 0, + }; + } + const summary = acc[event.testOpportunityCriteria]; + pushToSortedUniqueArray(summary.sellerIds, event.sellerId); + pushToSortedUniqueArray(summary.opportunityTypes, event.opportunityType); + pushToSortedUniqueArray(summary.bookingFlows, event.bookingFlow); + summary.numTestsFailing += 1; + return acc; + }, /** @type {MissingOpportunityDataSummary} */({})); + return this._missingOpportunityDataSummary; + } +} + +/** + * Push an item to an array that is already sorted. + * + * The item will be inserted into the array at a position which maintains sort + * order. If the item is already present in the array, it will not be added. + * + * ! Mutates `arr` + * + * @template T + * @param {T[]} arr + * @param {T} value + */ +function pushToSortedUniqueArray(arr, value) { + const index = _.sortedIndex(arr, value); + if (arr[index] === value) { + return; + } + arr.splice(index, 0, value); } module.exports = { diff --git a/packages/openactive-integration-tests/test/report-templates/summary.md.handlebars b/packages/openactive-integration-tests/test/report-templates/summary.md.handlebars index e34509e438..0dd2931b41 100644 --- a/packages/openactive-integration-tests/test/report-templates/summary.md.handlebars +++ b/packages/openactive-integration-tests/test/report-templates/summary.md.handlebars @@ -27,9 +27,50 @@ Mode: **{{ useRandomOpportunitiesMode }}** {{/if}}{{#if testResultSummary.numPendingTests}}– {{ testResultSummary.numPendingTests }} pending {{/if}} +{{#if missingOpportunityDataSummary}} + +## ❌ Missing Opportunity Data + +Some of the tests failed because there were not enough opportunities in the booking system under test that matched the required [criteria](https://github.com/openactive/openactive-test-suite/blob/master/packages/openactive-integration-tests/test/features/README.md). + +This only happens when using [Random Mode](https://developer.openactive.io/open-booking-api/test-suite#random-mode) as opposed to [Controlled Mode](https://developer.openactive.io/open-booking-api/test-suite#controlled-mode). + +In order to pass these tests, please increase the number of opportunities in the Booking System. Make sure that there are many opportunities available that match each criteria. + +You can use [test-data-generator](https://github.com/openactive/openactive-test-suite/tree/master/packages/openactive-integration-tests/test-data-generator) to get a specification of how much of each type of opportunity you need to generate. + +If any of these criteria are not relevant to your Booking System, you may need to remove the relevant feature from your Test Suite configuration (See [`implementedFeatures`](https://github.com/openactive/openactive-test-suite/tree/master/packages/openactive-integration-tests#implementedfeatures)). + +### Results + +This is a list of the criteria, opportunity type, etc for which no opportunities +were found, that also includes a list of which tests failed as a result: + +{{!-- This is rendered to HTML as the markdown->HTML renderer was creating

s + for each root list item, which resulted in overlarge margins --}} +

    + {{#each missingOpportunityDataSummary}} + +
  • + ❌️ Missing Criteria: {{@key}} +
      +
    • Opportunity Types: {{#each (lookup . "opportunityTypes")}}{{.}}{{#unless @last}}, {{/unless}}{{/each}}
      • +
    • Booking Flows: {{#each (lookup . "bookingFlows")}}{{.}}{{#unless @last}}, {{/unless}}{{/each}}
      • +
    • Sellers: {{#each (lookup . "sellerIds")}}{{.}}{{#unless @last}}, {{/unless}}{{/each}}
      • +
    • Tests failing as a result: {{lookup . "numTestsFailing"}}
      • +
    +
    • + + {{/each}} +
+ +{{/if}} + +## Tests + {{#eachSorted opportunityTypeGroups }} -{{{ opportunityTypeName }}} ---- + +### {{{ opportunityTypeName }}} {{#each featureGroups }} * {{{ validationIcon overallStatus }}} {{{ featureName }}} ({{implementedDisplayLabel}}) diff --git a/packages/openactive-integration-tests/tsconfig.json b/packages/openactive-integration-tests/tsconfig.json index ea05f45797..d038288a74 100644 --- a/packages/openactive-integration-tests/tsconfig.json +++ b/packages/openactive-integration-tests/tsconfig.json @@ -13,6 +13,7 @@ "test/**/*.d.ts", "test/features/**/*.js", "test/helpers/**/*.js", + "test/report-generator.js", "test/report-templates/**/*.js", "test/shared-behaviours/index.js", "test/shared-behaviours/errors.js",