From 95a2bb8f3dba168244f49ff57ca0532e085d6100 Mon Sep 17 00:00:00 2001 From: "Mx. Corey Frang" <gnarf37@gmail.com> Date: Thu, 4 Apr 2024 13:02:27 -0400 Subject: [PATCH 01/14] Per-Test status updates - basic queries and endpoints --- server/controllers/AutomationController.js | 144 ++++++++++++------ server/graphql-schema.js | 19 +++ ...240404171101-addCollectionJobTestStatus.js | 58 +++++++ server/models/CollectionJob.js | 6 + server/models/CollectionJobTestStatus.js | 56 +++++++ .../models/services/CollectionJobService.js | 87 ++++++++++- server/models/services/helpers.js | 8 +- server/resolvers/CollectionJob/index.js | 2 + .../CollectionJob/testStatusResolver.js | 15 ++ server/resolvers/index.js | 2 + server/routes/automation.js | 6 +- 11 files changed, 347 insertions(+), 56 deletions(-) create mode 100644 server/migrations/20240404171101-addCollectionJobTestStatus.js create mode 100644 server/models/CollectionJobTestStatus.js create mode 100644 server/resolvers/CollectionJob/index.js create mode 100644 server/resolvers/CollectionJob/testStatusResolver.js diff --git a/server/controllers/AutomationController.js b/server/controllers/AutomationController.js index 110351019..025c05aab 100644 --- a/server/controllers/AutomationController.js +++ b/server/controllers/AutomationController.js @@ -1,7 +1,8 @@ const axios = require('axios'); const { getCollectionJobById, - updateCollectionJobById + updateCollectionJobById, + updateCollectionJobTestStatusByQuery } = require('../models/services/CollectionJobService'); const { findOrCreateTestResult @@ -24,7 +25,7 @@ const { } = require('../models/services/TestResultReadService'); const http = require('http'); const { NO_OUTPUT_STRING } = require('../util/constants'); -const getTests = require('../models/services/TestsService'); +const runnableTestsResolver = require('../resolvers/TestPlanReport/runnableTestsResolver'); const getGraphQLContext = require('../graphql-context'); const httpAgent = new http.Agent({ family: 4 }); @@ -99,6 +100,23 @@ const updateJobStatus = async (req, res) => { ...(externalLogsUrl != null && { externalLogsUrl }) }; + // When new status is 'COMPLETED' or 'ERROR' or 'CANCELLED' + // update any CollectionJobTestStatus children still 'QUEUED' to be 'CANCELLED' + if ( + status === COLLECTION_JOB_STATUS.COMPLETED || + status === COLLECTION_JOB_STATUS.CANCELLED || + status === COLLECTION_JOB_STATUS.ERROR + ) { + await updateCollectionJobTestStatusByQuery({ + where: { + collectionJobId: req.params.jobID, + status: COLLECTION_JOB_STATUS.QUEUED + }, + values: { status: COLLECTION_JOB_STATUS.CANCELLED }, + transaction: req.transaction + }); + } + const graphqlResponse = await updateCollectionJobById({ id: req.params.jobID, values: updatePayload, @@ -137,32 +155,29 @@ const getApprovedFinalizedTestResults = async (testPlanRun, context) => { return getFinalizedTestResults({ testPlanReport, context }); }; -const updateOrCreateTestResultWithResponses = async ({ +const getTestByRowIdentifer = async ({ + testPlanRun, testRowIdentifier, + context +}) => { + const tests = await runnableTestsResolver( + testPlanRun.testPlanReport, + null, + context + ); + return tests.find( + test => parseInt(test.rowNumber, 10) === testRowIdentifier + ); +}; + +const updateOrCreateTestResultWithResponses = async ({ + testId, testPlanRun, responses, atVersionId, browserVersionId, context }) => { - const allTestsForTestPlanVersion = await getTests( - testPlanRun.testPlanReport.testPlanVersion - ); - - const isV2 = - testPlanRun.testPlanReport.testPlanVersion.metadata - .testFormatVersion === 2; - - const testId = allTestsForTestPlanVersion.find( - test => - (!isV2 || test.at?.name === 'NVDA') && - parseInt(test.rowNumber, 10) === testRowIdentifier - )?.id; - - if (testId === undefined) { - throwNoTestFoundError(testRowIdentifier); - } - const { testResult } = await findOrCreateTestResult({ testId, testPlanRunId: testPlanRun.id, @@ -243,9 +258,12 @@ const updateJobResults = async (req, res) => { const context = getGraphQLContext({ req }); const { transaction } = context; const { + // Old way - testCsvRow and presentationNumber can be removed + // once all requests from automation are using the new URL parameter testCsvRow, presentationNumber, responses, + status, capabilities: { atName, atVersion: atVersionName, @@ -263,35 +281,69 @@ const updateJobResults = async (req, res) => { `Job with id ${id} is not running, cannot update results` ); } - - /* TODO: Change this to use a better key based lookup system after gh-958 */ - const [at] = await getAts({ search: atName, transaction }); - const [browser] = await getBrowsers({ search: browserName, transaction }); - - const [atVersion, browserVersion] = await Promise.all([ - findOrCreateAtVersion({ - where: { atId: at.id, name: atVersionName }, - transaction - }), - findOrCreateBrowserVersion({ - where: { browserId: browser.id, name: browserVersionName }, - transaction + if (status && !Object.values(COLLECTION_JOB_STATUS).includes(status)) { + throw new HttpQueryError(400, `Invalid status: ${status}`, true); + } + const { testPlanRun } = job; + + // New way: testRowNumber is now a URL request param + // Old way: v1 tests store testCsvRow in rowNumber, v2 tests store presentationNumber in rowNumber + const testRowIdentifier = + req.params.testRowNumber ?? presentationNumber ?? testCsvRow; + const testId = ( + await getTestByRowIdentifer({ + testPlanRun, + testRowIdentifier, + context }) - ]); + )?.id; + + if (testId === undefined) { + throwNoTestFoundError(testRowIdentifier); + } - const processedResponses = convertEmptyStringsToNoOutputMessages(responses); + // status only update, or responses were provided (default to complete) + if (status || responses) { + await updateCollectionJobTestStatusByQuery({ + where: { collectionJobId: id, testId }, + // default to completed if not specified (when results are present) + values: { status: status ?? COLLECTION_JOB_STATUS.COMPLETED }, + transaction: req.transaction + }); + } - // v1 tests store testCsvRow in rowNumber, v2 tests store presentationNumber in rowNumber - const testRowIdentifier = presentationNumber ?? testCsvRow; + // responses were provided + if (responses) { + /* TODO: Change this to use a better key based lookup system after gh-958 */ + const [at] = await getAts({ search: atName, transaction }); + const [browser] = await getBrowsers({ + search: browserName, + transaction + }); - await updateOrCreateTestResultWithResponses({ - testRowIdentifier, - responses: processedResponses, - testPlanRun: job.testPlanRun, - atVersionId: atVersion.id, - browserVersionId: browserVersion.id, - context - }); + const [atVersion, browserVersion] = await Promise.all([ + findOrCreateAtVersion({ + where: { atId: at.id, name: atVersionName }, + transaction + }), + findOrCreateBrowserVersion({ + where: { browserId: browser.id, name: browserVersionName }, + transaction + }) + ]); + + const processedResponses = + convertEmptyStringsToNoOutputMessages(responses); + + await updateOrCreateTestResultWithResponses({ + testId, + responses: processedResponses, + testPlanRun, + atVersionId: atVersion.id, + browserVersionId: browserVersion.id, + context + }); + } res.json({ success: true }); }; diff --git a/server/graphql-schema.js b/server/graphql-schema.js index 92cf9d25a..e060abfa3 100644 --- a/server/graphql-schema.js +++ b/server/graphql-schema.js @@ -102,6 +102,25 @@ const graphqlSchema = gql` The URL where the logs for the job can be found. """ externalLogsUrl: String + """ + An array of individual test status for every runnable test in the Job. + """ + testStatus: [CollectionJobTestStatus] + } + + """ + A status for a specific Test on a specific CollectionJob. + """ + type CollectionJobTestStatus { + """ + The test this status reflects. + """ + test: Test! + """ + The status of the test, which can be "QUEUED", "RUNNING", "COMPLETED", + "ERROR", or "CANCELLED" + """ + status: CollectionJobStatus! } type Browser { diff --git a/server/migrations/20240404171101-addCollectionJobTestStatus.js b/server/migrations/20240404171101-addCollectionJobTestStatus.js new file mode 100644 index 000000000..7abf682ce --- /dev/null +++ b/server/migrations/20240404171101-addCollectionJobTestStatus.js @@ -0,0 +1,58 @@ +'use strict'; + +/** @type {import('sequelize-cli').Migration} */ +module.exports = { + async up(queryInterface, Sequelize) { + return queryInterface.sequelize.transaction(async transaction => { + await queryInterface.createTable( + 'CollectionJobTestStatus', + { + id: { + type: Sequelize.INTEGER, + primaryKey: true, + autoIncrement: true + }, + testId: { + type: Sequelize.STRING, + allowNull: false + }, + collectionJobId: { + type: Sequelize.INTEGER, + allowNull: false, + onDelete: 'CASCADE', + onUpdate: 'CASCADE', + references: { + model: 'CollectionJob', + key: 'id' + } + }, + status: { + type: Sequelize.STRING, + allowNull: false, + defaultValue: 'QUEUED' + } + }, + { transaction } + ); + await queryInterface.addConstraint('CollectionJobTestStatus', { + type: 'unique', + name: 'CollectionJob_Test_unique', + fields: ['collectionJobId', 'testId'], + transaction + }); + }); + }, + + async down(queryInterface) { + return queryInterface.sequelize.transaction(async transaction => { + await queryInterface.removeConstraint( + 'CollectionJobTestStatus', + 'CollectionJob_Test_unique', + { transaction } + ); + await queryInterface.dropTable('CollectionJobTestStatus', { + transaction + }); + }); + } +}; diff --git a/server/models/CollectionJob.js b/server/models/CollectionJob.js index c66499a80..dc57b229d 100644 --- a/server/models/CollectionJob.js +++ b/server/models/CollectionJob.js @@ -45,6 +45,12 @@ module.exports = function (sequelize, DataTypes) { sourceKey: 'testPlanRunId', as: 'testPlanRun' }); + + Model.hasMany(models.CollectionJobTestStatus, { + as: 'testStatus', + foreignKey: 'collectionJobId', + sourceKey: 'id' + }); }; Model.QUEUED = COLLECTION_JOB_STATUS.QUEUED; diff --git a/server/models/CollectionJobTestStatus.js b/server/models/CollectionJobTestStatus.js new file mode 100644 index 000000000..88427a1dd --- /dev/null +++ b/server/models/CollectionJobTestStatus.js @@ -0,0 +1,56 @@ +const { COLLECTION_JOB_STATUS } = require('../util/enums'); + +const MODEL_NAME = 'CollectionJobTestStatus'; + +module.exports = function (sequelize, DataTypes) { + const Model = sequelize.define( + MODEL_NAME, + { + id: { + type: DataTypes.INTEGER, + allowNull: false, + primaryKey: true, + autoIncrement: true + }, + collectionJobId: { + type: DataTypes.INTEGER, + allowNull: false, + references: { + model: 'CollectionJob', + key: 'id' + } + }, + testId: { + type: DataTypes.STRING, + allowNull: null + }, + status: { + type: DataTypes.STRING, + allowNull: false, + defaultValue: COLLECTION_JOB_STATUS.QUEUED + } + }, + { + timestamps: false, + tableName: MODEL_NAME + } + ); + + Model.associate = function (models) { + Model.belongsTo(models.CollectionJob, { + foreignKey: 'collectionJobId', + targetKey: 'id', + as: 'collectionJob', + onDelete: 'CASCADE', + onUpdate: 'CASCADE' + }); + }; + + Model.QUEUED = COLLECTION_JOB_STATUS.QUEUED; + Model.RUNNING = COLLECTION_JOB_STATUS.RUNNING; + Model.COMPLETED = COLLECTION_JOB_STATUS.COMPLETED; + Model.CANCELLED = COLLECTION_JOB_STATUS.CANCELLED; + Model.ERROR = COLLECTION_JOB_STATUS.ERROR; + + return Model; +}; diff --git a/server/models/services/CollectionJobService.js b/server/models/services/CollectionJobService.js index a73dbd20e..bbe756bd9 100644 --- a/server/models/services/CollectionJobService.js +++ b/server/models/services/CollectionJobService.js @@ -1,5 +1,5 @@ const ModelService = require('./ModelService'); -const { CollectionJob } = require('../'); +const { CollectionJob, CollectionJobTestStatus } = require('../'); const { COLLECTION_JOB_ATTRIBUTES, TEST_PLAN_ATTRIBUTES, @@ -8,7 +8,8 @@ const { TEST_PLAN_VERSION_ATTRIBUTES, AT_ATTRIBUTES, BROWSER_ATTRIBUTES, - USER_ATTRIBUTES + USER_ATTRIBUTES, + COLLECTION_JOB_TEST_STATUS_ATTRIBUTES } = require('./helpers'); const { COLLECTION_JOB_STATUS } = require('../../util/enums'); const { Op } = require('sequelize'); @@ -175,10 +176,21 @@ const userAssociation = userAttributes => ({ attributes: userAttributes }); +/** + * @param {string[]} collectionJobTestStatusAttributes - attributes to be returned in the result + * @returns {{association: string, attributes: string[]}} + */ +const collectionJobTestStatusAssociation = + collectionJobTestStatusAttributes => ({ + association: 'testStatus', + attributes: collectionJobTestStatusAttributes + }); + /** * @param {object} options * @param {object} options.values - CollectionJob to be created - * @param {string[]} options.collectionJobAttributes - TestPlanRun attributes to be returned in the result + * @param {string[]} options.collectionJobAttributes - CollectionJob attributes to be returned in the result + * @param {string[]} options.collectionJobTestStatusAttributes - CollectionJobTestStatus attributes to be returned in the result * @param {string[]} options.testPlanReportAttributes - TestPlanReport attributes to be returned in the result * @param {string[]} options.testPlanVersionAttributes - TestPlanVersion attributes to be returned in the result * @param {string[]} options.testPlanAttributes - TestPlanVersion attributes to be returned in the result @@ -195,6 +207,7 @@ const createCollectionJob = async ({ testPlanReportId }, collectionJobAttributes = COLLECTION_JOB_ATTRIBUTES, + collectionJobTestStatusAttributes = COLLECTION_JOB_TEST_STATUS_ATTRIBUTES, testPlanReportAttributes = TEST_PLAN_REPORT_ATTRIBUTES, testPlanRunAttributes = TEST_PLAN_RUN_ATTRIBUTES, testPlanVersionAttributes = TEST_PLAN_VERSION_ATTRIBUTES, @@ -222,10 +235,29 @@ const createCollectionJob = async ({ transaction }); + // create QUEUED status entries for each test in the test plan run + const context = getGraphQLContext({ req: { transaction } }); + const tests = await runnableTestsResolver( + testPlanRun.testPlanReport, + null, + context + ); + await ModelService.bulkCreate(CollectionJobTestStatus, { + valuesList: tests.map(test => ({ + testId: test.id, + collectionJobId: collectionJobResult.id, + status: COLLECTION_JOB_STATUS.QUEUED + })), + transaction + }); + return ModelService.getById(CollectionJob, { id: collectionJobResult.id, attributes: collectionJobAttributes, include: [ + collectionJobTestStatusAssociation( + collectionJobTestStatusAttributes + ), testPlanRunAssociation( testPlanRunAttributes, userAttributes, @@ -243,7 +275,8 @@ const createCollectionJob = async ({ /** * @param {object} options * @param {string} options.id - id for the CollectionJob - * @param {string[]} options.collectionJobAttributes - TestPlanRun attributes to be returned in the result + * @param {string[]} options.collectionJobAttributes - CollectionJob attributes to be returned in the result + * @param {string[]} options.collectionJobTestStatusAttributes - CollectionJobTestStatus attributes to be returned in the result * @param {string[]} options.testPlanReportAttributes - TestPlanReport attributes to be returned in the result * @param {string[]} options.testPlanVersionAttributes - TestPlanVersion attributes to be returned in the result * @param {string[]} options.testPlanAttributes - TestPlanVersion attributes to be returned in the result @@ -256,6 +289,7 @@ const createCollectionJob = async ({ const getCollectionJobById = async ({ id, collectionJobAttributes = COLLECTION_JOB_ATTRIBUTES, + collectionJobTestStatusAttributes = COLLECTION_JOB_TEST_STATUS_ATTRIBUTES, testPlanReportAttributes = TEST_PLAN_REPORT_ATTRIBUTES, testPlanRunAttributes = TEST_PLAN_RUN_ATTRIBUTES, testPlanVersionAttributes = TEST_PLAN_VERSION_ATTRIBUTES, @@ -269,6 +303,9 @@ const getCollectionJobById = async ({ id, attributes: collectionJobAttributes, include: [ + collectionJobTestStatusAssociation( + collectionJobTestStatusAttributes + ), testPlanRunAssociation( testPlanRunAttributes, userAttributes, @@ -287,7 +324,8 @@ const getCollectionJobById = async ({ * @param {object} options * @param {string|any} options.search - use this to combine with {@param filter} to be passed to Sequelize's where clause * @param {object} options.where - use this define conditions to be passed to Sequelize's where clause - * @param {string[]} options.collectionJobAttributes - Browser attributes to be returned in the result + * @param {string[]} options.collectionJobAttributes - CollectionJob attributes to be returned in the result + * @param {string[]} options.collectionJobTestStatusAttributes - CollectionJobTestStatus attributes to be returned in the result * @param {string[]} options.testPlanReportAttributes - TestPlanReport attributes to be returned in the result * @param {string[]} options.testPlanVersionAttributes - TestPlanVersion attributes to be returned in the result * @param {string[]} options.testPlanAttributes - TestPlanVersion attributes to be returned in the result @@ -306,6 +344,7 @@ const getCollectionJobs = async ({ search, where = {}, collectionJobAttributes = COLLECTION_JOB_ATTRIBUTES, + collectionJobTestStatusAttributes = COLLECTION_JOB_TEST_STATUS_ATTRIBUTES, testPlanReportAttributes = TEST_PLAN_REPORT_ATTRIBUTES, testPlanRunAttributes = TEST_PLAN_RUN_ATTRIBUTES, testPlanVersionAttributes = TEST_PLAN_VERSION_ATTRIBUTES, @@ -324,6 +363,9 @@ const getCollectionJobs = async ({ where, attributes: collectionJobAttributes, include: [ + collectionJobTestStatusAssociation( + collectionJobTestStatusAttributes + ), testPlanRunAssociation( testPlanRunAttributes, userAttributes, @@ -382,7 +424,8 @@ const triggerWorkflow = async (job, testIds, { transaction }) => { * @param {object} options * @param {string} options.id - id of the CollectionJob to be updated * @param {object} options.values - values to be used to update columns for the record being referenced for {@param id} - * @param {string[]} options.collectionJobAttributes - Browser attributes to be returned in the result + * @param {string[]} options.collectionJobAttributes - CollectionJob attributes to be returned in the result + * @param {string[]} options.collectionJobTestStatusAttributes - CollectionJobTestStatus attributes to be returned in the result * @param {string[]} options.testPlanReportAttributes - TestPlanReport attributes to be returned in the result * @param {string[]} options.testPlanVersionAttributes - TestPlanVersion attributes to be returned in the result * @param {string[]} options.testPlanAttributes - TestPlanVersion attributes to be returned in the result @@ -396,6 +439,7 @@ const updateCollectionJobById = async ({ id, values = {}, collectionJobAttributes = COLLECTION_JOB_ATTRIBUTES, + collectionJobTestStatusAttributes = COLLECTION_JOB_TEST_STATUS_ATTRIBUTES, testPlanReportAttributes = TEST_PLAN_REPORT_ATTRIBUTES, testPlanRunAttributes = TEST_PLAN_RUN_ATTRIBUTES, testPlanVersionAttributes = TEST_PLAN_VERSION_ATTRIBUTES, @@ -415,6 +459,9 @@ const updateCollectionJobById = async ({ id, attributes: collectionJobAttributes, include: [ + collectionJobTestStatusAssociation( + collectionJobTestStatusAttributes + ), testPlanRunAssociation( testPlanRunAttributes, userAttributes, @@ -592,6 +639,30 @@ const restartCollectionJob = async ({ id }, { transaction }) => { return triggerWorkflow(job, [], { transaction }); }; +/** + * CollectionJobTestStatus updates + */ + +/** + * Update CollectionJobTestStatus entries in bulk via query. + * @param {object} options + * @param {object} options.where - values of the CollectionJobTestStatus record to be updated + * @param {object} options.values - values to be used to update columns for the record being referenced + * @param {*} options.transaction - Sequelize transaction + * @returns {Promise<*>} + */ +const updateCollectionJobTestStatusByQuery = ({ + where, + values = {}, + transaction +}) => { + return ModelService.update(CollectionJobTestStatus, { + values, + where, + transaction + }); +}; + module.exports = { // Basic CRUD createCollectionJob, @@ -603,5 +674,7 @@ module.exports = { scheduleCollectionJob, restartCollectionJob, cancelCollectionJob, - retryCanceledCollections + retryCanceledCollections, + // Basic CRUD for CollectionJobTestStatus + updateCollectionJobTestStatusByQuery }; diff --git a/server/models/services/helpers.js b/server/models/services/helpers.js index 32c75b7cb..f7f188c71 100644 --- a/server/models/services/helpers.js +++ b/server/models/services/helpers.js @@ -11,7 +11,8 @@ const { User, UserRoles, UserAts, - CollectionJob + CollectionJob, + CollectionJobTestStatus } = require('../index'); /** @@ -39,5 +40,8 @@ module.exports = { USER_ATTRIBUTES: getSequelizeModelAttributes(User), USER_ROLES_ATTRIBUTES: getSequelizeModelAttributes(UserRoles), USER_ATS_ATTRIBUTES: getSequelizeModelAttributes(UserAts), - COLLECTION_JOB_ATTRIBUTES: getSequelizeModelAttributes(CollectionJob) + COLLECTION_JOB_ATTRIBUTES: getSequelizeModelAttributes(CollectionJob), + COLLECTION_JOB_TEST_STATUS_ATTRIBUTES: getSequelizeModelAttributes( + CollectionJobTestStatus + ) }; diff --git a/server/resolvers/CollectionJob/index.js b/server/resolvers/CollectionJob/index.js new file mode 100644 index 000000000..fa8c5525d --- /dev/null +++ b/server/resolvers/CollectionJob/index.js @@ -0,0 +1,2 @@ +const testStatus = require('./testStatusResolver'); +module.exports = { testStatus }; diff --git a/server/resolvers/CollectionJob/testStatusResolver.js b/server/resolvers/CollectionJob/testStatusResolver.js new file mode 100644 index 000000000..1c46a5d2b --- /dev/null +++ b/server/resolvers/CollectionJob/testStatusResolver.js @@ -0,0 +1,15 @@ +// eslint-disable-next-line no-unused-vars +const testStatusResolver = (collectionJob, _, context) => { + // resolve testStatus.test for each test + // testPlanRun "might" be null if the testPlanRun had been deleted + const testPlanTests = + collectionJob.testPlanRun?.testPlanReport.testPlanVersion.tests ?? []; + const tests = new Map(testPlanTests.map(test => [test.id, test])); + + return collectionJob.testStatus.map(status => ({ + ...status.dataValues, + // if not found, at least return the test id + test: tests.get(status.testId) ?? { id: status.testId } + })); +}; +module.exports = testStatusResolver; diff --git a/server/resolvers/index.js b/server/resolvers/index.js index 5e097b81d..5cf7dece6 100644 --- a/server/resolvers/index.js +++ b/server/resolvers/index.js @@ -41,6 +41,7 @@ const TestPlanRunOperations = require('./TestPlanRunOperations'); const TestResultOperations = require('./TestResultOperations'); const TestPlanVersionOperations = require('./TestPlanVersionOperations'); const CollectionJobOperations = require('./CollectionJobOperations'); +const CollectionJob = require('./CollectionJob'); const TestPlanRun = require('./TestPlanRun'); const Test = require('./Test'); const ScenarioResult = require('./ScenarioResult'); @@ -84,6 +85,7 @@ const resolvers = { AtOperations, AtVersionOperations, BrowserOperations, + CollectionJob, User, TestPlan, TestPlanVersion, diff --git a/server/routes/automation.js b/server/routes/automation.js index 460516af7..50bbe7605 100644 --- a/server/routes/automation.js +++ b/server/routes/automation.js @@ -10,10 +10,14 @@ const { handleError } = require('../middleware/handleError'); const router = Router(); +// Old Way (backwards compat) router.post('/:jobID/update', verifyAutomationScheduler, updateJobStatus); - router.post('/:jobID/result', verifyAutomationScheduler, updateJobResults); +// New way +router.post('/:jobID', verifyAutomationScheduler, updateJobStatus); +router.post('/:jobID/test/:testRowNumber', verifyAutomationScheduler, updateJobResults); + router.use(handleError); module.exports = router; From f5ea0657a7184ea2cbe7bbe2593dc92bf8a80333 Mon Sep 17 00:00:00 2001 From: "Mx. Corey Frang" <gnarf37@gmail.com> Date: Wed, 10 Apr 2024 14:50:29 -0400 Subject: [PATCH 02/14] updates for tests - new urls and checking testStatus --- server/controllers/AutomationController.js | 2 +- .../cancelCollectionJobResolver.js | 8 +++- server/routes/automation.js | 6 ++- .../integration/automation-scheduler.test.js | 45 +++++++++++++------ server/tests/integration/graphql.test.js | 4 +- 5 files changed, 47 insertions(+), 18 deletions(-) diff --git a/server/controllers/AutomationController.js b/server/controllers/AutomationController.js index 025c05aab..dfeb3bcaa 100644 --- a/server/controllers/AutomationController.js +++ b/server/controllers/AutomationController.js @@ -166,7 +166,7 @@ const getTestByRowIdentifer = async ({ context ); return tests.find( - test => parseInt(test.rowNumber, 10) === testRowIdentifier + test => parseInt(test.rowNumber, 10) === parseInt(testRowIdentifier, 10) ); }; diff --git a/server/resolvers/CollectionJobOperations/cancelCollectionJobResolver.js b/server/resolvers/CollectionJobOperations/cancelCollectionJobResolver.js index ea53c36e9..c7240ce50 100644 --- a/server/resolvers/CollectionJobOperations/cancelCollectionJobResolver.js +++ b/server/resolvers/CollectionJobOperations/cancelCollectionJobResolver.js @@ -2,7 +2,8 @@ const { AuthenticationError } = require('apollo-server'); const { updateCollectionJobById, - getCollectionJobById + getCollectionJobById, + updateCollectionJobTestStatusByQuery } = require('../../models/services/CollectionJobService'); const { COLLECTION_JOB_STATUS } = require('../../util/enums'); @@ -36,6 +37,11 @@ const cancelCollectionJobResolver = async ( if (collectionJob.status === COLLECTION_JOB_STATUS.COMPLETED) { return collectionJob; } else { + await updateCollectionJobTestStatusByQuery({ + where: { collectionJobId, status: COLLECTION_JOB_STATUS.QUEUED }, + values: { status: COLLECTION_JOB_STATUS.CANCELLED }, + transaction + }); return updateCollectionJobById({ id: collectionJobId, values: { status: COLLECTION_JOB_STATUS.CANCELLED }, diff --git a/server/routes/automation.js b/server/routes/automation.js index 50bbe7605..6ef659ef7 100644 --- a/server/routes/automation.js +++ b/server/routes/automation.js @@ -16,7 +16,11 @@ router.post('/:jobID/result', verifyAutomationScheduler, updateJobResults); // New way router.post('/:jobID', verifyAutomationScheduler, updateJobStatus); -router.post('/:jobID/test/:testRowNumber', verifyAutomationScheduler, updateJobResults); +router.post( + '/:jobID/test/:testRowNumber', + verifyAutomationScheduler, + updateJobResults +); router.use(handleError); diff --git a/server/tests/integration/automation-scheduler.test.js b/server/tests/integration/automation-scheduler.test.js index 09f5b209b..edcae553b 100644 --- a/server/tests/integration/automation-scheduler.test.js +++ b/server/tests/integration/automation-scheduler.test.js @@ -12,6 +12,7 @@ const markAsFinalResolver = require('../../resolvers/TestPlanReportOperations/ma const AtLoader = require('../../models/loaders/AtLoader'); const BrowserLoader = require('../../models/loaders/BrowserLoader'); const getGraphQLContext = require('../../graphql-context'); +const { COLLECTION_JOB_STATUS } = require('../../util/enums'); let mockAutomationSchedulerServer; let apiServer; @@ -138,6 +139,10 @@ const getTestCollectionJob = async (jobId, { transaction }) => } } } + testStatus { + test { id } + status + } } } `, @@ -166,6 +171,10 @@ const scheduleCollectionJobByMutation = async ({ transaction }) => } } } + testStatus { + test { id } + status + } } } `, @@ -288,7 +297,7 @@ describe('Automation controller', () => { }); }); - it('should cancel a job', async () => { + it('should cancel a job and all remaining tests', async () => { await dbCleaner(async transaction => { const { scheduleCollectionJob: job } = await scheduleCollectionJobByMutation({ transaction }); @@ -300,6 +309,9 @@ describe('Automation controller', () => { const { collectionJob: storedCollectionJob } = await getTestCollectionJob(job.id, { transaction }); expect(storedCollectionJob.status).toEqual('CANCELLED'); + for (const test of storedCollectionJob.testStatus) { + expect(test.status).toEqual('CANCELLED'); + } }); }); @@ -342,9 +354,7 @@ describe('Automation controller', () => { await dbCleaner(async transaction => { const { scheduleCollectionJob: job } = await scheduleCollectionJobByMutation({ transaction }); - const response = await sessionAgent.post( - `/api/jobs/${job.id}/update` - ); + const response = await sessionAgent.post(`/api/jobs/${job.id}`); expect(response.statusCode).toBe(403); expect(response.body).toEqual({ error: 'Unauthorized' @@ -357,7 +367,7 @@ describe('Automation controller', () => { const { scheduleCollectionJob: job } = await scheduleCollectionJobByMutation({ transaction }); const response = await sessionAgent - .post(`/api/jobs/${job.id}/update`) + .post(`/api/jobs/${job.id}`) .send({ status: 'INVALID' }) .set( 'x-automation-secret', @@ -372,7 +382,7 @@ describe('Automation controller', () => { it('should fail to update a job status for a non-existent jobId', async () => { const response = await sessionAgent - .post(`/api/jobs/${444}/update`) + .post(`/api/jobs/${444}`) .send({ status: 'RUNNING' }) .set( 'x-automation-secret', @@ -389,7 +399,7 @@ describe('Automation controller', () => { const { scheduleCollectionJob: job } = await scheduleCollectionJobByMutation({ transaction }); const response = await sessionAgent - .post(`/api/jobs/${job.id}/update`) + .post(`/api/jobs/${job.id}`) .send({ status: 'RUNNING' }) .set( 'x-automation-secret', @@ -421,7 +431,7 @@ describe('Automation controller', () => { const { scheduleCollectionJob: job } = await scheduleCollectionJobByMutation({ transaction }); const response = await sessionAgent - .post(`/api/jobs/${job.id}/update`) + .post(`/api/jobs/${job.id}`) .send({ status: 'CANCELLED', externalLogsUrl: 'https://www.aol.com/' @@ -462,7 +472,7 @@ describe('Automation controller', () => { transaction }); await sessionAgent - .post(`/api/jobs/${job.id}/update`) + .post(`/api/jobs/${job.id}`) .send({ status: 'RUNNING' }) .set( 'x-automation-secret', @@ -492,9 +502,8 @@ describe('Automation controller', () => { scenario => scenario.atId === at.id ).length; const response = await sessionAgent - .post(`/api/jobs/${job.id}/result`) + .post(`/api/jobs/${job.id}/test/${selectedTestRowNumber}`) .send({ - testCsvRow: selectedTestRowNumber, capabilities: { atName: at.name, atVersion: at.atVersions[0].name, @@ -543,6 +552,15 @@ describe('Automation controller', () => { ); }); }); + // also marks status for test as COMPLETED + const { collectionJob: storedCollectionJob } = + await getTestCollectionJob(job.id, { transaction }); + expect(storedCollectionJob.id).toEqual(job.id); + expect(storedCollectionJob.status).toEqual('RUNNING'); + const testStatus = storedCollectionJob.testStatus.find( + status => status.test.id === selectedTest.id + ); + expect(testStatus.status).toEqual(COLLECTION_JOB_STATUS.COMPLETED); }); }); @@ -588,7 +606,7 @@ describe('Automation controller', () => { transaction }); await sessionAgent - .post(`/api/jobs/${job.id}/update`) + .post(`/api/jobs/${job.id}`) .send({ status: 'RUNNING' }) .set( 'x-automation-secret', @@ -597,9 +615,8 @@ describe('Automation controller', () => { .set('x-transaction-id', transaction.id); const response = await sessionAgent - .post(`/api/jobs/${job.id}/result`) + .post(`/api/jobs/${job.id}/test/${selectedTestRowNumber}`) .send({ - testCsvRow: selectedTestRowNumber, capabilities: { atName: at.name, atVersion: atVersion.name, diff --git a/server/tests/integration/graphql.test.js b/server/tests/integration/graphql.test.js index 9bfeca0d8..01ab2fd1a 100644 --- a/server/tests/integration/graphql.test.js +++ b/server/tests/integration/graphql.test.js @@ -140,7 +140,8 @@ describe('graphql', () => { // 'TestResult' 'Issue', 'Vendor', - 'scheduleCollectionJob' + 'scheduleCollectionJob', + 'CollectionJobTestStatus' ]; const excludedTypeNameAndField = [ // Items formatted like this: @@ -159,6 +160,7 @@ describe('graphql', () => { ['Command', 'atOperatingMode'], // TODO: Include when v2 test format CI tests are done ['CollectionJob', 'testPlanRun'], ['CollectionJob', 'externalLogsUrl'], + ['CollectionJob', 'testStatus'], // These interact with Response Scheduler API // which is mocked in other tests. ['Mutation', 'scheduleCollectionJob'], From 79d0b73091fc03001208676059c19d56ef7a8f5d Mon Sep 17 00:00:00 2001 From: "Mx. Corey Frang" <gnarf37@gmail.com> Date: Wed, 10 Apr 2024 15:33:29 -0400 Subject: [PATCH 03/14] Add test for test-status-only update --- server/controllers/AutomationController.js | 2 +- .../integration/automation-scheduler.test.js | 53 +++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/server/controllers/AutomationController.js b/server/controllers/AutomationController.js index dfeb3bcaa..1860e46e3 100644 --- a/server/controllers/AutomationController.js +++ b/server/controllers/AutomationController.js @@ -269,7 +269,7 @@ const updateJobResults = async (req, res) => { atVersion: atVersionName, browserName, browserVersion: browserVersionName - } + } = {} } = req.body; const job = await getCollectionJobById({ id, transaction }); if (!job) { diff --git a/server/tests/integration/automation-scheduler.test.js b/server/tests/integration/automation-scheduler.test.js index edcae553b..d99272c7d 100644 --- a/server/tests/integration/automation-scheduler.test.js +++ b/server/tests/integration/automation-scheduler.test.js @@ -564,6 +564,59 @@ describe('Automation controller', () => { }); }); + it('should properly handle per-test status updates without capabilities present', async () => { + await apiServer.sessionAgentDbCleaner(async transaction => { + const { scheduleCollectionJob: job } = + await scheduleCollectionJobByMutation({ transaction }); + const collectionJob = await getCollectionJobById({ + id: job.id, + transaction + }); + // flag overall job as RUNNING + await sessionAgent + .post(`/api/jobs/${job.id}`) + .send({ status: 'RUNNING' }) + .set( + 'x-automation-secret', + process.env.AUTOMATION_SCHEDULER_SECRET + ) + .set('x-transaction-id', transaction.id); + + const { tests } = + collectionJob.testPlanRun.testPlanReport.testPlanVersion; + const selectedTestIndex = 0; + const selectedTestRowNumber = 1; + + const selectedTest = tests[selectedTestIndex]; + const response = await sessionAgent + .post(`/api/jobs/${job.id}/test/${selectedTestRowNumber}`) + .send({ + status: COLLECTION_JOB_STATUS.RUNNING + }) + .set( + 'x-automation-secret', + process.env.AUTOMATION_SCHEDULER_SECRET + ) + .set('x-transaction-id', transaction.id); + expect(response.statusCode).toBe(200); + + const { collectionJob: storedCollectionJob } = + await getTestCollectionJob(job.id, { transaction }); + expect(storedCollectionJob.id).toEqual(job.id); + expect(storedCollectionJob.status).toEqual('RUNNING'); + let foundStatus = false; + for (const testStatus of storedCollectionJob.testStatus) { + let expectedStatus = COLLECTION_JOB_STATUS.QUEUED; + if (testStatus.test.id === selectedTest.id) { + foundStatus = true; + expectedStatus = COLLECTION_JOB_STATUS.RUNNING; + } + expect(testStatus.status).toEqual(expectedStatus); + } + expect(foundStatus).toEqual(true); + }); + }); + it('should copy assertion results when updating with results that match historical results', async () => { await apiServer.sessionAgentDbCleaner(async transaction => { const context = getGraphQLContext({ From 0e16b81ca43b9506fd2e82efc1cf3067dd724bb4 Mon Sep 17 00:00:00 2001 From: "Mx. Corey Frang" <gnarf37@gmail.com> Date: Wed, 10 Apr 2024 16:05:18 -0400 Subject: [PATCH 04/14] more testing for per-test-status crud --- .../integration/automation-scheduler.test.js | 58 +++++++++++++++++-- 1 file changed, 54 insertions(+), 4 deletions(-) diff --git a/server/tests/integration/automation-scheduler.test.js b/server/tests/integration/automation-scheduler.test.js index d99272c7d..0dcd800da 100644 --- a/server/tests/integration/automation-scheduler.test.js +++ b/server/tests/integration/automation-scheduler.test.js @@ -573,9 +573,10 @@ describe('Automation controller', () => { transaction }); // flag overall job as RUNNING + const externalLogsUrl = 'https://example.com/test/log/url'; await sessionAgent .post(`/api/jobs/${job.id}`) - .send({ status: 'RUNNING' }) + .send({ status: 'RUNNING', externalLogsUrl }) .set( 'x-automation-secret', process.env.AUTOMATION_SCHEDULER_SECRET @@ -586,9 +587,8 @@ describe('Automation controller', () => { collectionJob.testPlanRun.testPlanReport.testPlanVersion; const selectedTestIndex = 0; const selectedTestRowNumber = 1; - const selectedTest = tests[selectedTestIndex]; - const response = await sessionAgent + let response = await sessionAgent .post(`/api/jobs/${job.id}/test/${selectedTestRowNumber}`) .send({ status: COLLECTION_JOB_STATUS.RUNNING @@ -600,10 +600,13 @@ describe('Automation controller', () => { .set('x-transaction-id', transaction.id); expect(response.statusCode).toBe(200); - const { collectionJob: storedCollectionJob } = + let { collectionJob: storedCollectionJob } = await getTestCollectionJob(job.id, { transaction }); expect(storedCollectionJob.id).toEqual(job.id); expect(storedCollectionJob.status).toEqual('RUNNING'); + expect(storedCollectionJob.externalLogsUrl).toEqual( + externalLogsUrl + ); let foundStatus = false; for (const testStatus of storedCollectionJob.testStatus) { let expectedStatus = COLLECTION_JOB_STATUS.QUEUED; @@ -614,6 +617,53 @@ describe('Automation controller', () => { expect(testStatus.status).toEqual(expectedStatus); } expect(foundStatus).toEqual(true); + + // check that putting this test into ERROR and sending an overall + // collection job ERROR will properly update things + response = await sessionAgent + .post(`/api/jobs/${job.id}/test/${selectedTestRowNumber}`) + .send({ + status: COLLECTION_JOB_STATUS.ERROR + }) + .set( + 'x-automation-secret', + process.env.AUTOMATION_SCHEDULER_SECRET + ) + .set('x-transaction-id', transaction.id); + expect(response.statusCode).toBe(200); + + response = await sessionAgent + .post(`/api/jobs/${job.id}`) + .send({ + // avoiding sending externalLogsUrl here to test that when + // missing it is not overwritten/emptied. + status: COLLECTION_JOB_STATUS.ERROR + }) + .set( + 'x-automation-secret', + process.env.AUTOMATION_SCHEDULER_SECRET + ) + .set('x-transaction-id', transaction.id); + expect(response.statusCode).toBe(200); + + storedCollectionJob = ( + await getTestCollectionJob(job.id, { transaction }) + ).collectionJob; + expect(storedCollectionJob.id).toEqual(job.id); + expect(storedCollectionJob.status).toEqual('ERROR'); + expect(storedCollectionJob.externalLogsUrl).toEqual( + externalLogsUrl + ); + foundStatus = false; + for (const testStatus of storedCollectionJob.testStatus) { + let expectedStatus = COLLECTION_JOB_STATUS.CANCELLED; + if (testStatus.test.id === selectedTest.id) { + foundStatus = true; + expectedStatus = COLLECTION_JOB_STATUS.ERROR; + } + expect(testStatus.status).toEqual(expectedStatus); + } + expect(foundStatus).toEqual(true); }); }); From d699e844972d85f67a20301f718499516d59b8f6 Mon Sep 17 00:00:00 2001 From: "Mx. Corey Frang" <gnarf37@gmail.com> Date: Wed, 10 Apr 2024 16:18:15 -0400 Subject: [PATCH 05/14] add tests for old urls/format --- .../integration/automation-scheduler.test.js | 106 ++++++++++++++++++ 1 file changed, 106 insertions(+) diff --git a/server/tests/integration/automation-scheduler.test.js b/server/tests/integration/automation-scheduler.test.js index 0dcd800da..d4b4dae36 100644 --- a/server/tests/integration/automation-scheduler.test.js +++ b/server/tests/integration/automation-scheduler.test.js @@ -564,6 +564,112 @@ describe('Automation controller', () => { }); }); + // Remove this test entirely when we get rid of the old URLs + it('BACKCOMPAT: should properly handle per-test status updates with old urls', async () => { + await apiServer.sessionAgentDbCleaner(async transaction => { + const { scheduleCollectionJob: job } = + await scheduleCollectionJobByMutation({ transaction }); + const collectionJob = await getCollectionJobById({ + id: job.id, + transaction + }); + // flag overall job as RUNNING + const externalLogsUrl = 'https://example.com/test/log/url'; + await sessionAgent + .post(`/api/jobs/${job.id}/update`) + .send({ status: 'RUNNING', externalLogsUrl }) + .set( + 'x-automation-secret', + process.env.AUTOMATION_SCHEDULER_SECRET + ) + .set('x-transaction-id', transaction.id); + + const { tests } = + collectionJob.testPlanRun.testPlanReport.testPlanVersion; + const selectedTestIndex = 0; + const selectedTestRowNumber = 1; + const selectedTest = tests[selectedTestIndex]; + let response = await sessionAgent + .post(`/api/jobs/${job.id}/result`) + .send({ + testCsvRow: selectedTestRowNumber, + status: COLLECTION_JOB_STATUS.RUNNING + }) + .set( + 'x-automation-secret', + process.env.AUTOMATION_SCHEDULER_SECRET + ) + .set('x-transaction-id', transaction.id); + expect(response.statusCode).toBe(200); + + let { collectionJob: storedCollectionJob } = + await getTestCollectionJob(job.id, { transaction }); + expect(storedCollectionJob.id).toEqual(job.id); + expect(storedCollectionJob.status).toEqual('RUNNING'); + expect(storedCollectionJob.externalLogsUrl).toEqual( + externalLogsUrl + ); + let foundStatus = false; + for (const testStatus of storedCollectionJob.testStatus) { + let expectedStatus = COLLECTION_JOB_STATUS.QUEUED; + if (testStatus.test.id === selectedTest.id) { + foundStatus = true; + expectedStatus = COLLECTION_JOB_STATUS.RUNNING; + } + expect(testStatus.status).toEqual(expectedStatus); + } + expect(foundStatus).toEqual(true); + + // check that putting this test into ERROR and sending an overall + // collection job ERROR will properly update things + response = await sessionAgent + .post(`/api/jobs/${job.id}/result`) + .send({ + testCsvRow: selectedTestRowNumber, + status: COLLECTION_JOB_STATUS.ERROR + }) + .set( + 'x-automation-secret', + process.env.AUTOMATION_SCHEDULER_SECRET + ) + .set('x-transaction-id', transaction.id); + expect(response.statusCode).toBe(200); + + response = await sessionAgent + .post(`/api/jobs/${job.id}/update`) + .send({ + // avoiding sending externalLogsUrl here to test that when + // missing it is not overwritten/emptied. + status: COLLECTION_JOB_STATUS.ERROR + }) + .set( + 'x-automation-secret', + process.env.AUTOMATION_SCHEDULER_SECRET + ) + .set('x-transaction-id', transaction.id); + expect(response.statusCode).toBe(200); + + storedCollectionJob = ( + await getTestCollectionJob(job.id, { transaction }) + ).collectionJob; + expect(storedCollectionJob.id).toEqual(job.id); + expect(storedCollectionJob.status).toEqual('ERROR'); + expect(storedCollectionJob.externalLogsUrl).toEqual( + externalLogsUrl + ); + foundStatus = false; + for (const testStatus of storedCollectionJob.testStatus) { + let expectedStatus = COLLECTION_JOB_STATUS.CANCELLED; + if (testStatus.test.id === selectedTest.id) { + foundStatus = true; + expectedStatus = COLLECTION_JOB_STATUS.ERROR; + } + expect(testStatus.status).toEqual(expectedStatus); + } + expect(foundStatus).toEqual(true); + }); + }); + it('should properly handle per-test status updates without capabilities present', async () => { await apiServer.sessionAgentDbCleaner(async transaction => { const { scheduleCollectionJob: job } = From e945cb9f3866b2f6dad12496772c7942079b8512 Mon Sep 17 00:00:00 2001 From: "Mx. Corey Frang" <gnarf37@gmail.com> Date: Wed, 24 Apr 2024 10:38:18 -0400 Subject: [PATCH 06/14] Working queries and updates to get BotRunTestStatusList to work --- .../components/BotRunTestStatusList/index.js | 142 +++++++----------- .../BotRunTestStatusList/queries.js | 15 +- server/graphql-schema.js | 4 + server/resolvers/CollectionJob/index.js | 3 +- .../CollectionJob/testPlanRunResolver.js | 8 + .../TestPlanRun/collectionJobResolver.js | 15 ++ server/resolvers/TestPlanRun/index.js | 3 +- 7 files changed, 91 insertions(+), 99 deletions(-) create mode 100644 server/resolvers/CollectionJob/testPlanRunResolver.js create mode 100644 server/resolvers/TestPlanRun/collectionJobResolver.js diff --git a/client/components/BotRunTestStatusList/index.js b/client/components/BotRunTestStatusList/index.js index 4fb00a729..cbd5d28c3 100644 --- a/client/components/BotRunTestStatusList/index.js +++ b/client/components/BotRunTestStatusList/index.js @@ -1,13 +1,9 @@ -import React, { useEffect, useMemo, useRef, useState } from 'react'; +import React, { useMemo } from 'react'; import PropTypes from 'prop-types'; -import { - COLLECTION_JOB_STATUS_BY_TEST_PLAN_RUN_ID_QUERY, - TEST_PLAN_RUNS_TEST_RESULTS_QUERY -} from './queries'; -import { useLazyQuery, useQuery } from '@apollo/client'; +import { TEST_PLAN_RUNS_TEST_RESULTS_QUERY } from './queries'; +import { useQuery } from '@apollo/client'; import styled from '@emotion/styled'; import ReportStatusDot from '../common/ReportStatusDot'; -import { isBot } from '../../utils/automation'; const BotRunTestStatusUnorderedList = styled.ul` list-style-type: none; @@ -35,7 +31,9 @@ const BotRunTestStatusUnorderedList = styled.ul` const testCountString = (count, status) => `${count} Test${count === 1 ? '' : 's'} ${status}`; -const BotRunTestStatusList = ({ testPlanReportId, runnableTestsLength }) => { +const pollInterval = 2000; + +const BotRunTestStatusList = ({ testPlanReportId }) => { const { data: testPlanRunsQueryResult, startPolling, @@ -43,105 +41,74 @@ const BotRunTestStatusList = ({ testPlanReportId, runnableTestsLength }) => { } = useQuery(TEST_PLAN_RUNS_TEST_RESULTS_QUERY, { variables: { testPlanReportId }, fetchPolicy: 'cache-and-network', - pollInterval: 2000 + pollInterval }); - const [getCollectionJobStatus, { data: collectionJobStatusQueryResult }] = - useLazyQuery(COLLECTION_JOB_STATUS_BY_TEST_PLAN_RUN_ID_QUERY, { - fetchPolicy: 'cache-and-network' - }); - - const [collectedData, setCollectedData] = useState([]); - const requestedTestRunIds = useRef(new Set()); - - const botTestPlanRuns = useMemo(() => { - if (!testPlanRunsQueryResult?.testPlanRuns) { - return []; - } - return testPlanRunsQueryResult.testPlanRuns.filter(testPlanRun => - isBot(testPlanRun.tester) - ); - }, [testPlanRunsQueryResult?.testPlanRuns]); - - useEffect(() => { - const ids = botTestPlanRuns.map(run => run.id); - for (const id of ids) { - if (!requestedTestRunIds.current.has(id)) { - requestedTestRunIds.current.add(id); - getCollectionJobStatus({ - variables: { testPlanRunId: id } - }); - } - } - }, [botTestPlanRuns]); - - useEffect(() => { - if (collectionJobStatusQueryResult?.collectionJobByTestPlanRunId) { - const { status } = - collectionJobStatusQueryResult.collectionJobByTestPlanRunId; - setCollectedData(prev => [...prev, status]); - } - }, [collectionJobStatusQueryResult?.collectionJobByTestPlanRunId]); - - const [numTestsCompleted, numTestsQueued, numTestsCancelled] = - useMemo(() => { - const res = [0, 0, 0]; - if ( - botTestPlanRuns && - botTestPlanRuns.length && - collectedData.length === botTestPlanRuns.length - ) { - for (let i = 0; i < botTestPlanRuns.length; i++) { - const status = collectedData[i]; - res[0] += botTestPlanRuns[i].testResults.length; - switch (status) { - case 'COMPLETED': - case 'RUNNING': - case 'QUEUED': - res[1] += - runnableTestsLength - - botTestPlanRuns[i].testResults.length; - break; - case 'CANCELLED': - res[2] += - runnableTestsLength - - botTestPlanRuns[i].testResults.length; - break; - default: - break; + const { COMPLETED, ERROR, RUNNING, CANCELLED, QUEUED } = useMemo(() => { + const counter = { + COMPLETED: 0, + ERROR: 0, + RUNNING: 0, + CANCELLED: 0, + QUEUED: 0 + }; + let anyPossibleUpdates = false; + if (testPlanRunsQueryResult?.testPlanRuns) { + for (const { + collectionJob + } of testPlanRunsQueryResult.testPlanRuns) { + if (collectionJob?.testStatus) { + for (const { status } of collectionJob.testStatus) { + counter[status]++; + if (status === 'QUEUED' || status === 'RUNNING') { + anyPossibleUpdates = true; + } } } - if ( - res[0] + res[2] === - runnableTestsLength * botTestPlanRuns.length - ) { - stopPolling(); - } } - return res; - }, [testPlanRunsQueryResult, collectedData, stopPolling, startPolling]); + // it's possible that we got incomplete data on first fetch and + // stopped the polling, so restart the polling if we detect any + // possible future updates, otherwise stop. + if (anyPossibleUpdates) { + startPolling(pollInterval); + } else { + stopPolling(); + } + } + return counter; + }, [testPlanRunsQueryResult, stopPolling, startPolling]); if ( - !botTestPlanRuns || - botTestPlanRuns.length === 0 || - !collectedData || - !(collectedData.length === botTestPlanRuns.length) + !testPlanRunsQueryResult || + testPlanRunsQueryResult.testPlanRuns.length === 0 ) { return null; } return ( <BotRunTestStatusUnorderedList className="text-secondary fs-6"> + {ERROR > 0 && ( + <li className="m-2"> + <ReportStatusDot className="tests-error" /> + {testCountString(ERROR, 'Error')} + </li> + )} <li className="m-2"> <ReportStatusDot className="tests-complete" /> - {testCountString(numTestsCompleted, 'Completed')} + {testCountString(COMPLETED, 'Completed')} </li> + {RUNNING > 0 && ( + <li className="m-2"> + <ReportStatusDot className="tests-running" /> + {testCountString(RUNNING, 'Running')} + </li> + )} <li className="m-2"> <ReportStatusDot className="tests-queued" /> - {testCountString(numTestsQueued, 'Queued')} + {testCountString(QUEUED, 'Queued')} </li> <li className="m-2"> <ReportStatusDot className="tests-cancelled" /> - {testCountString(numTestsCancelled, 'Cancelled')} + {testCountString(CANCELLED, 'Cancelled')} </li> </BotRunTestStatusUnorderedList> ); @@ -149,7 +116,6 @@ const BotRunTestStatusList = ({ testPlanReportId, runnableTestsLength }) => { BotRunTestStatusList.propTypes = { testPlanReportId: PropTypes.string.isRequired, - runnableTestsLength: PropTypes.number.isRequired }; export default BotRunTestStatusList; diff --git a/client/components/BotRunTestStatusList/queries.js b/client/components/BotRunTestStatusList/queries.js index 048acc1aa..cd99fb2bc 100644 --- a/client/components/BotRunTestStatusList/queries.js +++ b/client/components/BotRunTestStatusList/queries.js @@ -16,15 +16,12 @@ export const TEST_PLAN_RUNS_TEST_RESULTS_QUERY = gql` } } } - } - } -`; - -export const COLLECTION_JOB_STATUS_BY_TEST_PLAN_RUN_ID_QUERY = gql` - query CollectionJobStatusByTestPlanRunId($testPlanRunId: ID!) { - collectionJobByTestPlanRunId(testPlanRunId: $testPlanRunId) { - id - status + collectionJob { + status + testStatus { + status + } + } } } `; diff --git a/server/graphql-schema.js b/server/graphql-schema.js index e060abfa3..647607d99 100644 --- a/server/graphql-schema.js +++ b/server/graphql-schema.js @@ -846,6 +846,10 @@ const graphqlSchema = gql` Whether the TestPlanRun was initiated by the Response Collection System """ initiatedByAutomation: Boolean! + """ + The CollectionJob related to this testPlanRun + """ + collectionJob: CollectionJob } """ diff --git a/server/resolvers/CollectionJob/index.js b/server/resolvers/CollectionJob/index.js index fa8c5525d..64347cf91 100644 --- a/server/resolvers/CollectionJob/index.js +++ b/server/resolvers/CollectionJob/index.js @@ -1,2 +1,3 @@ const testStatus = require('./testStatusResolver'); -module.exports = { testStatus }; +const testPlanRun = require('./testPlanRunResolver'); +module.exports = { testStatus, testPlanRun }; diff --git a/server/resolvers/CollectionJob/testPlanRunResolver.js b/server/resolvers/CollectionJob/testPlanRunResolver.js new file mode 100644 index 000000000..c9a67d494 --- /dev/null +++ b/server/resolvers/CollectionJob/testPlanRunResolver.js @@ -0,0 +1,8 @@ +// eslint-disable-next-line no-unused-vars +const testPlanRunResolver = (collectionJob, _, context) => { + if (collectionJob.__testPlanRunChild) { + throw new Error('can not request TestPlanRun.collectionJob.testPlanRun'); + } + return collectionJob.testPlanRun; +}; +module.exports = testPlanRunResolver; diff --git a/server/resolvers/TestPlanRun/collectionJobResolver.js b/server/resolvers/TestPlanRun/collectionJobResolver.js new file mode 100644 index 000000000..12058a8c3 --- /dev/null +++ b/server/resolvers/TestPlanRun/collectionJobResolver.js @@ -0,0 +1,15 @@ +const collectionJobByTestPlanRunIdResolver = require('../collectionJobByTestPlanRunIdResolver'); + +const collectionJobResolver = async ( + testPlanRun, + args, // eslint-disable-line no-unused-vars + context // eslint-disable-line no-unused-vars +) => { + const collectionJob = await collectionJobByTestPlanRunIdResolver(null, { testPlanRunId: testPlanRun.id }, context); + if (collectionJob) { + return {...collectionJob.dataValues, __testPlanRunChild: true}; + } + return collectionJob; +}; + +module.exports = collectionJobResolver; diff --git a/server/resolvers/TestPlanRun/index.js b/server/resolvers/TestPlanRun/index.js index 74770a793..f542d26d4 100644 --- a/server/resolvers/TestPlanRun/index.js +++ b/server/resolvers/TestPlanRun/index.js @@ -1,7 +1,8 @@ const testResults = require('./testResultsResolver'); const testResultsLength = require('./testResultsLengthResolver'); - +const collectionJob = require('./collectionJobResolver'); module.exports = { + collectionJob, testResults, testResultsLength }; From 6d1d1851861e436979c53df3c963e63ccf059a1d Mon Sep 17 00:00:00 2001 From: "Mx. Corey Frang" <gnarf37@gmail.com> Date: Wed, 24 Apr 2024 11:23:05 -0400 Subject: [PATCH 07/14] more updates for test queue page view --- .../components/BotRunTestStatusList/index.js | 24 +++--- .../common/ReportStatusDot/index.jsx | 13 +++- .../CollectionJob/testPlanRunResolver.js | 4 +- .../TestPlanRun/collectionJobResolver.js | 8 +- .../util/mock-automation-scheduler-server.js | 76 ++++++++++++------- 5 files changed, 84 insertions(+), 41 deletions(-) diff --git a/client/components/BotRunTestStatusList/index.js b/client/components/BotRunTestStatusList/index.js index cbd5d28c3..6d1873c50 100644 --- a/client/components/BotRunTestStatusList/index.js +++ b/client/components/BotRunTestStatusList/index.js @@ -86,6 +86,12 @@ const BotRunTestStatusList = ({ testPlanReportId }) => { } return ( <BotRunTestStatusUnorderedList className="text-secondary fs-6"> + {RUNNING > 0 && ( + <li className="m-2"> + <ReportStatusDot className="tests-running" /> + {testCountString(RUNNING, 'Running')} + </li> + )} {ERROR > 0 && ( <li className="m-2"> <ReportStatusDot className="tests-error" /> @@ -96,26 +102,22 @@ const BotRunTestStatusList = ({ testPlanReportId }) => { <ReportStatusDot className="tests-complete" /> {testCountString(COMPLETED, 'Completed')} </li> - {RUNNING > 0 && ( - <li className="m-2"> - <ReportStatusDot className="tests-running" /> - {testCountString(RUNNING, 'Running')} - </li> - )} <li className="m-2"> <ReportStatusDot className="tests-queued" /> {testCountString(QUEUED, 'Queued')} </li> - <li className="m-2"> - <ReportStatusDot className="tests-cancelled" /> - {testCountString(CANCELLED, 'Cancelled')} - </li> + {CANCELLED > 0 && ( + <li className="m-2"> + <ReportStatusDot className="tests-cancelled" /> + {testCountString(CANCELLED, 'Cancelled')} + </li> + )} </BotRunTestStatusUnorderedList> ); }; BotRunTestStatusList.propTypes = { - testPlanReportId: PropTypes.string.isRequired, + testPlanReportId: PropTypes.string.isRequired }; export default BotRunTestStatusList; diff --git a/client/components/common/ReportStatusDot/index.jsx b/client/components/common/ReportStatusDot/index.jsx index d369e91ad..0c44c6521 100644 --- a/client/components/common/ReportStatusDot/index.jsx +++ b/client/components/common/ReportStatusDot/index.jsx @@ -17,6 +17,15 @@ const ReportStatusDot = styled.span` background: #7c7c7c; } + &.tests-running { + border: 2px solid #1e8f37; + background: #d2d5d9; + } + + &.tests-error { + background: #e3261f; + } + &.tests-queued, &.reports-in-progress { background: #3876e8; @@ -27,7 +36,9 @@ const ReportStatusDot = styled.span` background: #2ba51c; } - &.tests-cancelled, + &.tests-cancelled { + background: #a231ff; + } &.reports-missing { background: #ce1b4c; } diff --git a/server/resolvers/CollectionJob/testPlanRunResolver.js b/server/resolvers/CollectionJob/testPlanRunResolver.js index c9a67d494..62014f8bd 100644 --- a/server/resolvers/CollectionJob/testPlanRunResolver.js +++ b/server/resolvers/CollectionJob/testPlanRunResolver.js @@ -1,7 +1,9 @@ // eslint-disable-next-line no-unused-vars const testPlanRunResolver = (collectionJob, _, context) => { if (collectionJob.__testPlanRunChild) { - throw new Error('can not request TestPlanRun.collectionJob.testPlanRun'); + throw new Error( + 'can not request TestPlanRun.collectionJob.testPlanRun' + ); } return collectionJob.testPlanRun; }; diff --git a/server/resolvers/TestPlanRun/collectionJobResolver.js b/server/resolvers/TestPlanRun/collectionJobResolver.js index 12058a8c3..ecfb6552c 100644 --- a/server/resolvers/TestPlanRun/collectionJobResolver.js +++ b/server/resolvers/TestPlanRun/collectionJobResolver.js @@ -5,9 +5,13 @@ const collectionJobResolver = async ( args, // eslint-disable-line no-unused-vars context // eslint-disable-line no-unused-vars ) => { - const collectionJob = await collectionJobByTestPlanRunIdResolver(null, { testPlanRunId: testPlanRun.id }, context); + const collectionJob = await collectionJobByTestPlanRunIdResolver( + null, + { testPlanRunId: testPlanRun.id }, + context + ); if (collectionJob) { - return {...collectionJob.dataValues, __testPlanRunChild: true}; + return { ...collectionJob.dataValues, __testPlanRunChild: true }; } return collectionJob; }; diff --git a/server/tests/util/mock-automation-scheduler-server.js b/server/tests/util/mock-automation-scheduler-server.js index eeec2dd23..87f267fff 100644 --- a/server/tests/util/mock-automation-scheduler-server.js +++ b/server/tests/util/mock-automation-scheduler-server.js @@ -12,6 +12,9 @@ const { } = require('../../middleware/transactionMiddleware'); const { query } = require('../util/graphql-test-utilities'); +// 0 = no chance of test errors, 1 = always errors +const TEST_ERROR_CHANCE = 0; + const setupMockAutomationSchedulerServer = async () => { const app = express(); app.use(express.json()); @@ -26,9 +29,22 @@ const setupMockAutomationSchedulerServer = async () => { shutdownManager = new GracefulShutdownManager(listener); }); + const timeout = ms => + new Promise(resolve => setTimeout(() => resolve(), ms)); + const simulateJobStatusUpdate = async (jobId, newStatus) => { await axios.post( - `${process.env.APP_SERVER}/api/jobs/${jobId}/update`, + `${process.env.APP_SERVER}/api/jobs/${jobId}`, + { + status: newStatus + }, + axiosConfig + ); + }; + + const simulateTestStatusUpdate = async (jobId, testId, newStatus) => { + await axios.post( + `${process.env.APP_SERVER}/api/jobs/${jobId}/test/${testId}`, { status: newStatus }, @@ -66,22 +82,37 @@ const setupMockAutomationSchedulerServer = async () => { responses }; - testResult[isV2 ? 'presentationNumber' : 'testCsvRow'] = - currentTest.rowNumber; try { - await axios.post( - `${process.env.APP_SERVER}/api/jobs/${jobId}/result`, - testResult, - axiosConfig + await simulateTestStatusUpdate( + jobId, + currentTest.rowNumber, + COLLECTION_JOB_STATUS.RUNNING ); - } catch (e) { - // Likely just means the test was cancelled - return; - } + await timeout(Math.random() * 2000); + + if (Math.random() < TEST_ERROR_CHANCE) { + await simulateTestStatusUpdate( + jobId, + currentTest.rowNumber, + COLLECTION_JOB_STATUS.ERROR + ); + return simulateJobStatusUpdate( + jobId, + COLLECTION_JOB_STATUS.ERROR + ); + } else { + testResult[isV2 ? 'presentationNumber' : 'testCsvRow'] = + currentTest.rowNumber; + await axios.post( + `${process.env.APP_SERVER}/api/jobs/${jobId}/test/${currentTest.rowNumber}`, + testResult, + axiosConfig + ); + } - if (currentTestIndex < tests.length - 1) { - setTimeout(() => { - simulateResultCompletion( + if (currentTestIndex < tests.length - 1) { + await timeout(Math.random() * 5000); + return simulateResultCompletion( tests, atName, atVersionName, @@ -91,19 +122,12 @@ const setupMockAutomationSchedulerServer = async () => { currentTestIndex + 1, isV2 ); - }, Math.random() * 5000); - } else { - setTimeout( - () => - simulateJobStatusUpdate( - jobId, - COLLECTION_JOB_STATUS.COMPLETED - ), - 1000 - ); + } else { + simulateJobStatusUpdate(jobId, COLLECTION_JOB_STATUS.COMPLETED); + } + } catch (error) { + console.error('Error simulating collection job', error); } - - return testResult; }; app.post('/jobs/new', async (req, res) => { From b2c6927642ce90f7c1b344ee274f7eb509c20f73 Mon Sep 17 00:00:00 2001 From: "Mx. Corey Frang" <gnarf37@gmail.com> Date: Wed, 24 Apr 2024 12:11:46 -0400 Subject: [PATCH 08/14] update tests to pass --- server/tests/integration/graphql.test.js | 3 ++- server/tests/util/mock-automation-scheduler-server.js | 2 -- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/server/tests/integration/graphql.test.js b/server/tests/integration/graphql.test.js index 01ab2fd1a..efe2b6b0a 100644 --- a/server/tests/integration/graphql.test.js +++ b/server/tests/integration/graphql.test.js @@ -506,10 +506,11 @@ describe('graphql', () => { releasedAt } } - testPlanRun(id: 3) { + testPlanRun(id: 1) { __typename id initiatedByAutomation + collectionJob { id } testPlanReport { id } diff --git a/server/tests/util/mock-automation-scheduler-server.js b/server/tests/util/mock-automation-scheduler-server.js index 87f267fff..ca9c65ec1 100644 --- a/server/tests/util/mock-automation-scheduler-server.js +++ b/server/tests/util/mock-automation-scheduler-server.js @@ -101,8 +101,6 @@ const setupMockAutomationSchedulerServer = async () => { COLLECTION_JOB_STATUS.ERROR ); } else { - testResult[isV2 ? 'presentationNumber' : 'testCsvRow'] = - currentTest.rowNumber; await axios.post( `${process.env.APP_SERVER}/api/jobs/${jobId}/test/${currentTest.rowNumber}`, testResult, From ff36385772819ba79b7dc9625339f170e5f20e0f Mon Sep 17 00:00:00 2001 From: "Mx. Corey Frang" <gnarf37@gmail.com> Date: Wed, 24 Apr 2024 12:44:06 -0400 Subject: [PATCH 09/14] ensure recursive query fails --- server/tests/integration/graphql.test.js | 25 ++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/server/tests/integration/graphql.test.js b/server/tests/integration/graphql.test.js index efe2b6b0a..20f8a4bf6 100644 --- a/server/tests/integration/graphql.test.js +++ b/server/tests/integration/graphql.test.js @@ -784,6 +784,31 @@ describe('graphql', () => { ); }); + // esure recursive query of collectionJob<>testPlanRun fails at some depth + await expect( + typeAwareQuery( + gql` + query { + collectionJob(id: 1) { + id + testPlanRun { + id + collectionJob { + id + testPlanRun { + id + } + } + } + } + } + `, + { + transaction: false + } + ) + ).rejects.toBeDefined(); + expect(() => { const missingTypes = checkForMissingTypes(); if (missingTypes.length) { From c3f4d741229a21a3971899016460536f03c9d093 Mon Sep 17 00:00:00 2001 From: "Mx. Corey Frang" <gnarf37@gmail.com> Date: Wed, 1 May 2024 10:50:13 -0400 Subject: [PATCH 10/14] update to new urls --- server/controllers/AutomationController.js | 43 +++++++++------------- server/services/GithubWorkflowService.js | 4 +- server/util/enums.js | 8 +++- 3 files changed, 26 insertions(+), 29 deletions(-) diff --git a/server/controllers/AutomationController.js b/server/controllers/AutomationController.js index 1860e46e3..c5e63bee8 100644 --- a/server/controllers/AutomationController.js +++ b/server/controllers/AutomationController.js @@ -18,7 +18,7 @@ const { findOrCreateBrowserVersion } = require('../models/services/BrowserService'); const { HttpQueryError } = require('apollo-server-core'); -const { COLLECTION_JOB_STATUS } = require('../util/enums'); +const { COLLECTION_JOB_STATUS, isJobStatusFinal } = require('../util/enums'); const populateData = require('../services/PopulatedData/populateData'); const { getFinalizedTestResults @@ -100,13 +100,9 @@ const updateJobStatus = async (req, res) => { ...(externalLogsUrl != null && { externalLogsUrl }) }; - // When new status is 'COMPLETED' or 'ERROR' or 'CANCELLED' + // When new status is considered "final" ('COMPLETED' or 'ERROR' or 'CANCELLED') // update any CollectionJobTestStatus children still 'QUEUED' to be 'CANCELLED' - if ( - status === COLLECTION_JOB_STATUS.COMPLETED || - status === COLLECTION_JOB_STATUS.CANCELLED || - status === COLLECTION_JOB_STATUS.ERROR - ) { + if (isJobStatusFinal(status)) { await updateCollectionJobTestStatusByQuery({ where: { collectionJobId: req.params.jobID, @@ -155,19 +151,13 @@ const getApprovedFinalizedTestResults = async (testPlanRun, context) => { return getFinalizedTestResults({ testPlanReport, context }); }; -const getTestByRowIdentifer = async ({ - testPlanRun, - testRowIdentifier, - context -}) => { +const getTestByRowNumber = async ({ testPlanRun, testRowNumber, context }) => { const tests = await runnableTestsResolver( testPlanRun.testPlanReport, null, context ); - return tests.find( - test => parseInt(test.rowNumber, 10) === parseInt(testRowIdentifier, 10) - ); + return tests.find(test => test.rowNumber === parseInt(testRowNumber, 10)); }; const updateOrCreateTestResultWithResponses = async ({ @@ -258,10 +248,6 @@ const updateJobResults = async (req, res) => { const context = getGraphQLContext({ req }); const { transaction } = context; const { - // Old way - testCsvRow and presentationNumber can be removed - // once all requests from automation are using the new URL parameter - testCsvRow, - presentationNumber, responses, status, capabilities: { @@ -271,6 +257,15 @@ const updateJobResults = async (req, res) => { browserVersion: browserVersionName } = {} } = req.body; + + // The testRowNumber is now a URL request param, previously it was passed + // in the request body as `testCsvRow` for v1 tests, `presentationNumber` + // for v2 tests. + const testRowNumber = + req.params.testRowNumber ?? + req.body.presentationNumber ?? + req.body.testCsvRow; + const job = await getCollectionJobById({ id, transaction }); if (!job) { throwNoJobFoundError(id); @@ -286,20 +281,16 @@ const updateJobResults = async (req, res) => { } const { testPlanRun } = job; - // New way: testRowNumber is now a URL request param - // Old way: v1 tests store testCsvRow in rowNumber, v2 tests store presentationNumber in rowNumber - const testRowIdentifier = - req.params.testRowNumber ?? presentationNumber ?? testCsvRow; const testId = ( - await getTestByRowIdentifer({ + await getTestByRowNumber({ testPlanRun, - testRowIdentifier, + testRowNumber, context }) )?.id; if (testId === undefined) { - throwNoTestFoundError(testRowIdentifier); + throwNoTestFoundError(testRowNumber); } // status only update, or responses were provided (default to complete) diff --git a/server/services/GithubWorkflowService.js b/server/services/GithubWorkflowService.js index 9d21765d7..2fe571563 100644 --- a/server/services/GithubWorkflowService.js +++ b/server/services/GithubWorkflowService.js @@ -128,8 +128,8 @@ const createGithubWorkflow = async ({ job, directory, gitSha }) => { ); const browser = job.testPlanRun.testPlanReport.browser.name.toLowerCase(); const inputs = { - callback_url: `https://${callbackUrlHostname}/api/jobs/${job.id}/result`, - status_url: `https://${callbackUrlHostname}/api/jobs/${job.id}/update`, + callback_url: `https://${callbackUrlHostname}/api/jobs/${job.id}/test/:testRowNumber`, + status_url: `https://${callbackUrlHostname}/api/jobs/${job.id}`, callback_header: `x-automation-secret:${process.env.AUTOMATION_SCHEDULER_SECRET}`, work_dir: `tests/${directory}`, test_pattern: '{reference/**,test-*-nvda.*}', diff --git a/server/util/enums.js b/server/util/enums.js index ce07982d2..56ab4c579 100644 --- a/server/util/enums.js +++ b/server/util/enums.js @@ -6,6 +6,12 @@ const COLLECTION_JOB_STATUS = { CANCELLED: 'CANCELLED' }; +const isJobStatusFinal = status => + status === COLLECTION_JOB_STATUS.COMPLETED || + status === COLLECTION_JOB_STATUS.CANCELLED || + status === COLLECTION_JOB_STATUS.ERROR; + module.exports = { - COLLECTION_JOB_STATUS + COLLECTION_JOB_STATUS, + isJobStatusFinal }; From cf50111b84d333ebe6ed0c11dab644808ffd0f12 Mon Sep 17 00:00:00 2001 From: "Mx. Corey Frang" <gnarf37@gmail.com> Date: Wed, 1 May 2024 11:00:48 -0400 Subject: [PATCH 11/14] Remove back-compat URLs --- server/controllers/AutomationController.js | 14 +-- server/routes/automation.js | 5 - .../integration/automation-scheduler.test.js | 106 ------------------ 3 files changed, 4 insertions(+), 121 deletions(-) diff --git a/server/controllers/AutomationController.js b/server/controllers/AutomationController.js index c5e63bee8..39f2ba930 100644 --- a/server/controllers/AutomationController.js +++ b/server/controllers/AutomationController.js @@ -157,7 +157,9 @@ const getTestByRowNumber = async ({ testPlanRun, testRowNumber, context }) => { null, context ); - return tests.find(test => test.rowNumber === parseInt(testRowNumber, 10)); + return tests.find( + test => parseInt(test.rowNumber, 10) === parseInt(testRowNumber, 10) + ); }; const updateOrCreateTestResultWithResponses = async ({ @@ -244,7 +246,7 @@ const updateOrCreateTestResultWithResponses = async ({ }; const updateJobResults = async (req, res) => { - const id = req.params.jobID; + const { jobID: id, testRowNumber } = req.params; const context = getGraphQLContext({ req }); const { transaction } = context; const { @@ -258,14 +260,6 @@ const updateJobResults = async (req, res) => { } = {} } = req.body; - // The testRowNumber is now a URL request param, previously it was passed - // in the request body as `testCsvRow` for v1 tests, `presentationNumber` - // for v2 tests. - const testRowNumber = - req.params.testRowNumber ?? - req.body.presentationNumber ?? - req.body.testCsvRow; - const job = await getCollectionJobById({ id, transaction }); if (!job) { throwNoJobFoundError(id); diff --git a/server/routes/automation.js b/server/routes/automation.js index 6ef659ef7..5d6540e64 100644 --- a/server/routes/automation.js +++ b/server/routes/automation.js @@ -10,11 +10,6 @@ const { handleError } = require('../middleware/handleError'); const router = Router(); -// Old Way (backwards compat) -router.post('/:jobID/update', verifyAutomationScheduler, updateJobStatus); -router.post('/:jobID/result', verifyAutomationScheduler, updateJobResults); - -// New way router.post('/:jobID', verifyAutomationScheduler, updateJobStatus); router.post( '/:jobID/test/:testRowNumber', diff --git a/server/tests/integration/automation-scheduler.test.js b/server/tests/integration/automation-scheduler.test.js index d4b4dae36..0dcd800da 100644 --- a/server/tests/integration/automation-scheduler.test.js +++ b/server/tests/integration/automation-scheduler.test.js @@ -564,112 +564,6 @@ describe('Automation controller', () => { }); }); - // Remove this test entirely when we get rid of the old URLs - it('BACKCOMPAT: should properly handle per-test status updates with old urls', async () => { - await apiServer.sessionAgentDbCleaner(async transaction => { - const { scheduleCollectionJob: job } = - await scheduleCollectionJobByMutation({ transaction }); - const collectionJob = await getCollectionJobById({ - id: job.id, - transaction - }); - // flag overall job as RUNNING - const externalLogsUrl = 'https://example.com/test/log/url'; - await sessionAgent - .post(`/api/jobs/${job.id}/update`) - .send({ status: 'RUNNING', externalLogsUrl }) - .set( - 'x-automation-secret', - process.env.AUTOMATION_SCHEDULER_SECRET - ) - .set('x-transaction-id', transaction.id); - - const { tests } = - collectionJob.testPlanRun.testPlanReport.testPlanVersion; - const selectedTestIndex = 0; - const selectedTestRowNumber = 1; - const selectedTest = tests[selectedTestIndex]; - let response = await sessionAgent - .post(`/api/jobs/${job.id}/result`) - .send({ - testCsvRow: selectedTestRowNumber, - status: COLLECTION_JOB_STATUS.RUNNING - }) - .set( - 'x-automation-secret', - process.env.AUTOMATION_SCHEDULER_SECRET - ) - .set('x-transaction-id', transaction.id); - expect(response.statusCode).toBe(200); - - let { collectionJob: storedCollectionJob } = - await getTestCollectionJob(job.id, { transaction }); - expect(storedCollectionJob.id).toEqual(job.id); - expect(storedCollectionJob.status).toEqual('RUNNING'); - expect(storedCollectionJob.externalLogsUrl).toEqual( - externalLogsUrl - ); - let foundStatus = false; - for (const testStatus of storedCollectionJob.testStatus) { - let expectedStatus = COLLECTION_JOB_STATUS.QUEUED; - if (testStatus.test.id === selectedTest.id) { - foundStatus = true; - expectedStatus = COLLECTION_JOB_STATUS.RUNNING; - } - expect(testStatus.status).toEqual(expectedStatus); - } - expect(foundStatus).toEqual(true); - - // check that putting this test into ERROR and sending an overall - // collection job ERROR will properly update things - response = await sessionAgent - .post(`/api/jobs/${job.id}/result`) - .send({ - testCsvRow: selectedTestRowNumber, - status: COLLECTION_JOB_STATUS.ERROR - }) - .set( - 'x-automation-secret', - process.env.AUTOMATION_SCHEDULER_SECRET - ) - .set('x-transaction-id', transaction.id); - expect(response.statusCode).toBe(200); - - response = await sessionAgent - .post(`/api/jobs/${job.id}/update`) - .send({ - // avoiding sending externalLogsUrl here to test that when - // missing it is not overwritten/emptied. - status: COLLECTION_JOB_STATUS.ERROR - }) - .set( - 'x-automation-secret', - process.env.AUTOMATION_SCHEDULER_SECRET - ) - .set('x-transaction-id', transaction.id); - expect(response.statusCode).toBe(200); - - storedCollectionJob = ( - await getTestCollectionJob(job.id, { transaction }) - ).collectionJob; - expect(storedCollectionJob.id).toEqual(job.id); - expect(storedCollectionJob.status).toEqual('ERROR'); - expect(storedCollectionJob.externalLogsUrl).toEqual( - externalLogsUrl - ); - foundStatus = false; - for (const testStatus of storedCollectionJob.testStatus) { - let expectedStatus = COLLECTION_JOB_STATUS.CANCELLED; - if (testStatus.test.id === selectedTest.id) { - foundStatus = true; - expectedStatus = COLLECTION_JOB_STATUS.ERROR; - } - expect(testStatus.status).toEqual(expectedStatus); - } - expect(foundStatus).toEqual(true); - }); - }); - it('should properly handle per-test status updates without capabilities present', async () => { await apiServer.sessionAgentDbCleaner(async transaction => { const { scheduleCollectionJob: job } = From 292527db36f770c431aa11ea2e007416a2b85789 Mon Sep 17 00:00:00 2001 From: "Mx. Corey Frang" <gnarf37@gmail.com> Date: Wed, 1 May 2024 11:40:37 -0400 Subject: [PATCH 12/14] testing tests on github --- client/tests/BotRunTestStatusList.test.jsx | 269 ++++++++++----------- 1 file changed, 128 insertions(+), 141 deletions(-) diff --git a/client/tests/BotRunTestStatusList.test.jsx b/client/tests/BotRunTestStatusList.test.jsx index 66b93e03d..db3e38520 100644 --- a/client/tests/BotRunTestStatusList.test.jsx +++ b/client/tests/BotRunTestStatusList.test.jsx @@ -5,13 +5,11 @@ import React from 'react'; import { render, waitFor } from '@testing-library/react'; import { MockedProvider } from '@apollo/client/testing'; import BotRunTestStatusList from '../components/BotRunTestStatusList'; -import { - TEST_PLAN_RUNS_TEST_RESULTS_QUERY, - COLLECTION_JOB_STATUS_BY_TEST_PLAN_RUN_ID_QUERY -} from '../components/BotRunTestStatusList/queries'; +import { TEST_PLAN_RUNS_TEST_RESULTS_QUERY } from '../components/BotRunTestStatusList/queries'; import '@testing-library/jest-dom/extend-expect'; +import { COLLECTION_JOB_STATUS } from '../../server/util/enums'; -const getMocks = (testPlanRuns, collectionJobStatuses) => { +const getMocks = testPlanRuns => { const testPlanRunMock = { request: { query: TEST_PLAN_RUNS_TEST_RESULTS_QUERY, @@ -20,22 +18,7 @@ const getMocks = (testPlanRuns, collectionJobStatuses) => { result: { data: { testPlanRuns } } }; - const collectionJobStatusMocks = testPlanRuns.map((testRun, index) => ({ - request: { - query: COLLECTION_JOB_STATUS_BY_TEST_PLAN_RUN_ID_QUERY, - variables: { testPlanRunId: testRun.id } - }, - result: { - data: { - collectionJobByTestPlanRunId: { - status: collectionJobStatuses[index], - id: testRun.id - } - } - } - })); - - return [testPlanRunMock, ...collectionJobStatusMocks]; + return [testPlanRunMock]; }; test('correctly displays statuses for single COMPLETED test run', async () => { @@ -43,137 +26,141 @@ test('correctly displays statuses for single COMPLETED test run', async () => { { id: '0', testResults: new Array(3).fill(null), - tester: { username: 'bot' } - } - ]; - - const collectionJobStatuses = ['COMPLETED']; - - const mocks = getMocks(testPlanRuns, collectionJobStatuses); - - const { getByText } = render( - <MockedProvider mocks={mocks} addTypename={false}> - <BotRunTestStatusList - testPlanReportId="1" - runnableTestsLength={3} - /> - </MockedProvider> - ); - - await waitFor(() => { - expect(getByText('3 Tests Completed')).toBeInTheDocument(); - expect(getByText('0 Tests Queued')).toBeInTheDocument(); - expect(getByText('0 Tests Cancelled')).toBeInTheDocument(); - }); -}); - -test('correctly ignores test results from a human-submitted test plan run', async () => { - const testPlanRuns = [ - { - id: '0', - testResults: new Array(2).fill(null), - tester: { username: 'bot' } - }, - { - id: '1', - testResults: new Array(2).fill(null), - tester: { username: 'human' } + tester: { username: 'bot' }, + collectionJob: { + status: COLLECTION_JOB_STATUS.COMPLETED, + testStatus: [ + { status: COLLECTION_JOB_STATUS.COMPLETED }, + { status: COLLECTION_JOB_STATUS.COMPLETED }, + { status: COLLECTION_JOB_STATUS.COMPLETED } + ] + } } ]; - const collectionJobStatuses = ['COMPLETED', 'COMPLETED']; + const mocks = getMocks(testPlanRuns); - const mocks = getMocks(testPlanRuns, collectionJobStatuses); - - const { getByText } = render( + const screen = render( <MockedProvider mocks={mocks} addTypename={false}> - <BotRunTestStatusList - testPlanReportId="1" - runnableTestsLength={2} - /> + <BotRunTestStatusList testPlanReportId="1" /> </MockedProvider> ); - await waitFor(async () => { - expect(getByText('2 Tests Completed')).toBeInTheDocument(); - expect(getByText('0 Tests Queued')).toBeInTheDocument(); - expect(getByText('0 Tests Cancelled')).toBeInTheDocument(); - }); -}); - -test('correctly displays statuses for CANCELLED test run', async () => { - const testPlanRuns = [ - { - id: '0', - testResults: new Array(2).fill(null), - tester: { username: 'bot' } - } - ]; - - const collectionJobStatuses = ['CANCELLED']; - - const mocks = getMocks(testPlanRuns, collectionJobStatuses); - - const { getByText } = render( - <MockedProvider mocks={mocks} addTypename={false}> - <BotRunTestStatusList - testPlanReportId="1" - runnableTestsLength={3} - /> - </MockedProvider> - ); + const { getByText } = screen; await waitFor(() => { - expect(getByText('2 Tests Completed')).toBeInTheDocument(); + expect(getByText('3 Tests Completed')).toBeInTheDocument(); expect(getByText('0 Tests Queued')).toBeInTheDocument(); - expect(getByText('1 Test Cancelled')).toBeInTheDocument(); }); }); -test('correctly displays statuses for multiple RUNNING and QUEUED test runs', async () => { - const testPlanRuns = [ - { - id: '0', - testResults: new Array(2).fill(null), - tester: { username: 'bot' } - }, - { - id: '1', - testResults: new Array(2).fill(null), - tester: { username: 'bot' } - }, - { - id: '2', - testResults: [null], - tester: { username: 'bot' } - }, - { - id: '3', - testResults: new Array(2).fill(null), - tester: { username: 'human' } - } - ]; - - const collectionJobStatuses = ['RUNNING', 'RUNNING', 'CANCELLED']; - - const mocks = getMocks(testPlanRuns, collectionJobStatuses); - - const { getByText } = render( - <MockedProvider mocks={mocks} addTypename={false}> - <BotRunTestStatusList - testPlanReportId="1" - runnableTestsLength={3} - /> - </MockedProvider> - ); - - await waitFor(async () => { - // Wait for the component to update - // Imperfect but prevents needing to detect loading removal - await setTimeout(() => { - expect(getByText('2 Tests Completed')).toBeInTheDocument(); - expect(getByText('1 Test Queued')).toBeInTheDocument(); - expect(getByText('1 Test Cancelled')).toBeInTheDocument(); - }, 500); - }); -}); +// test('correctly ignores test results from a human-submitted test plan run', async () => { +// const testPlanRuns = [ +// { +// id: '0', +// testResults: new Array(2).fill(null), +// tester: { username: 'bot' } +// }, +// { +// id: '1', +// testResults: new Array(2).fill(null), +// tester: { username: 'human' } +// } +// ]; + +// const collectionJobStatuses = ['COMPLETED', 'COMPLETED']; + +// const mocks = getMocks(testPlanRuns, collectionJobStatuses); + +// const { getByText } = render( +// <MockedProvider mocks={mocks} addTypename={false}> +// <BotRunTestStatusList +// testPlanReportId="1" +// runnableTestsLength={2} +// /> +// </MockedProvider> +// ); + +// await waitFor(async () => { +// expect(getByText('2 Tests Completed')).toBeInTheDocument(); +// expect(getByText('0 Tests Queued')).toBeInTheDocument(); +// expect(getByText('0 Tests Cancelled')).toBeInTheDocument(); +// }); +// }); + +// test('correctly displays statuses for CANCELLED test run', async () => { +// const testPlanRuns = [ +// { +// id: '0', +// testResults: new Array(2).fill(null), +// tester: { username: 'bot' } +// } +// ]; + +// const collectionJobStatuses = ['CANCELLED']; + +// const mocks = getMocks(testPlanRuns, collectionJobStatuses); + +// const { getByText } = render( +// <MockedProvider mocks={mocks} addTypename={false}> +// <BotRunTestStatusList +// testPlanReportId="1" +// runnableTestsLength={3} +// /> +// </MockedProvider> +// ); + +// await waitFor(() => { +// expect(getByText('2 Tests Completed')).toBeInTheDocument(); +// expect(getByText('0 Tests Queued')).toBeInTheDocument(); +// expect(getByText('1 Test Cancelled')).toBeInTheDocument(); +// }); +// }); + +// test('correctly displays statuses for multiple RUNNING and QUEUED test runs', async () => { +// const testPlanRuns = [ +// { +// id: '0', +// testResults: new Array(2).fill(null), +// tester: { username: 'bot' } +// }, +// { +// id: '1', +// testResults: new Array(2).fill(null), +// tester: { username: 'bot' } +// }, +// { +// id: '2', +// testResults: [null], +// tester: { username: 'bot' } +// }, +// { +// id: '3', +// testResults: new Array(2).fill(null), +// tester: { username: 'human' } +// } +// ]; + +// const collectionJobStatuses = ['RUNNING', 'RUNNING', 'CANCELLED']; + +// const mocks = getMocks(testPlanRuns, collectionJobStatuses); + +// const { getByText } = render( +// <MockedProvider mocks={mocks} addTypename={false}> +// <BotRunTestStatusList +// testPlanReportId="1" +// runnableTestsLength={3} +// /> +// </MockedProvider> +// ); + +// await waitFor(async () => { +// // Wait for the component to update +// // Imperfect but prevents needing to detect loading removal +// await setTimeout(() => { +// expect(getByText('2 Tests Completed')).toBeInTheDocument(); +// expect(getByText('1 Test Queued')).toBeInTheDocument(); +// expect(getByText('1 Test Cancelled')).toBeInTheDocument(); +// }, 500); +// }); +// }); From 6d4ce9c12253ba83b8128c88d5c10a392fdda6a8 Mon Sep 17 00:00:00 2001 From: "Mx. Corey Frang" <gnarf37@gmail.com> Date: Wed, 1 May 2024 11:53:31 -0400 Subject: [PATCH 13/14] remaining tests --- client/tests/BotRunTestStatusList.test.jsx | 228 +++++++++++---------- 1 file changed, 118 insertions(+), 110 deletions(-) diff --git a/client/tests/BotRunTestStatusList.test.jsx b/client/tests/BotRunTestStatusList.test.jsx index db3e38520..dfb428703 100644 --- a/client/tests/BotRunTestStatusList.test.jsx +++ b/client/tests/BotRunTestStatusList.test.jsx @@ -54,113 +54,121 @@ test('correctly displays statuses for single COMPLETED test run', async () => { }); }); -// test('correctly ignores test results from a human-submitted test plan run', async () => { -// const testPlanRuns = [ -// { -// id: '0', -// testResults: new Array(2).fill(null), -// tester: { username: 'bot' } -// }, -// { -// id: '1', -// testResults: new Array(2).fill(null), -// tester: { username: 'human' } -// } -// ]; - -// const collectionJobStatuses = ['COMPLETED', 'COMPLETED']; - -// const mocks = getMocks(testPlanRuns, collectionJobStatuses); - -// const { getByText } = render( -// <MockedProvider mocks={mocks} addTypename={false}> -// <BotRunTestStatusList -// testPlanReportId="1" -// runnableTestsLength={2} -// /> -// </MockedProvider> -// ); - -// await waitFor(async () => { -// expect(getByText('2 Tests Completed')).toBeInTheDocument(); -// expect(getByText('0 Tests Queued')).toBeInTheDocument(); -// expect(getByText('0 Tests Cancelled')).toBeInTheDocument(); -// }); -// }); - -// test('correctly displays statuses for CANCELLED test run', async () => { -// const testPlanRuns = [ -// { -// id: '0', -// testResults: new Array(2).fill(null), -// tester: { username: 'bot' } -// } -// ]; - -// const collectionJobStatuses = ['CANCELLED']; - -// const mocks = getMocks(testPlanRuns, collectionJobStatuses); - -// const { getByText } = render( -// <MockedProvider mocks={mocks} addTypename={false}> -// <BotRunTestStatusList -// testPlanReportId="1" -// runnableTestsLength={3} -// /> -// </MockedProvider> -// ); - -// await waitFor(() => { -// expect(getByText('2 Tests Completed')).toBeInTheDocument(); -// expect(getByText('0 Tests Queued')).toBeInTheDocument(); -// expect(getByText('1 Test Cancelled')).toBeInTheDocument(); -// }); -// }); - -// test('correctly displays statuses for multiple RUNNING and QUEUED test runs', async () => { -// const testPlanRuns = [ -// { -// id: '0', -// testResults: new Array(2).fill(null), -// tester: { username: 'bot' } -// }, -// { -// id: '1', -// testResults: new Array(2).fill(null), -// tester: { username: 'bot' } -// }, -// { -// id: '2', -// testResults: [null], -// tester: { username: 'bot' } -// }, -// { -// id: '3', -// testResults: new Array(2).fill(null), -// tester: { username: 'human' } -// } -// ]; - -// const collectionJobStatuses = ['RUNNING', 'RUNNING', 'CANCELLED']; - -// const mocks = getMocks(testPlanRuns, collectionJobStatuses); - -// const { getByText } = render( -// <MockedProvider mocks={mocks} addTypename={false}> -// <BotRunTestStatusList -// testPlanReportId="1" -// runnableTestsLength={3} -// /> -// </MockedProvider> -// ); - -// await waitFor(async () => { -// // Wait for the component to update -// // Imperfect but prevents needing to detect loading removal -// await setTimeout(() => { -// expect(getByText('2 Tests Completed')).toBeInTheDocument(); -// expect(getByText('1 Test Queued')).toBeInTheDocument(); -// expect(getByText('1 Test Cancelled')).toBeInTheDocument(); -// }, 500); -// }); -// }); +test('correctly ignores test results from a human-submitted test plan run', async () => { + const testPlanRuns = [ + { + id: '0', + testResults: new Array(2).fill(null), + tester: { username: 'bot' }, + collectionJob: { + status: COLLECTION_JOB_STATUS.COMPLETED, + testStatus: [ + { status: COLLECTION_JOB_STATUS.COMPLETED }, + { status: COLLECTION_JOB_STATUS.COMPLETED }, + { status: COLLECTION_JOB_STATUS.COMPLETED } + ] + } + }, + { + id: '1', + testResults: new Array(2).fill(null), + tester: { username: 'human' }, + collectionJob: null + } + ]; + + const mocks = getMocks(testPlanRuns); + + const { getByText } = render( + <MockedProvider mocks={mocks} addTypename={false}> + <BotRunTestStatusList testPlanReportId="1" /> + </MockedProvider> + ); + + await waitFor(async () => { + expect(getByText('2 Tests Completed')).toBeInTheDocument(); + expect(getByText('0 Tests Queued')).toBeInTheDocument(); + }); +}); + +test('correctly displays statuses for CANCELLED test run', async () => { + const testPlanRuns = [ + { + id: '0', + testResults: new Array(2).fill(null), + tester: { username: 'bot' }, + collectionJob: { + status: COLLECTION_JOB_STATUS.CANCELLED, + testStatus: [ + { status: COLLECTION_JOB_STATUS.COMPLETED }, + { status: COLLECTION_JOB_STATUS.COMPLETED }, + { status: COLLECTION_JOB_STATUS.CANCELLED } + ] + } + } + ]; + + const mocks = getMocks(testPlanRuns); + + const { getByText } = render( + <MockedProvider mocks={mocks} addTypename={false}> + <BotRunTestStatusList testPlanReportId="1" /> + </MockedProvider> + ); + + await waitFor(() => { + expect(getByText('2 Tests Completed')).toBeInTheDocument(); + expect(getByText('0 Tests Queued')).toBeInTheDocument(); + expect(getByText('1 Test Cancelled')).toBeInTheDocument(); + }); +}); + +test('correctly displays statuses for multiple RUNNING and QUEUED test runs', async () => { + const testPlanRuns = [ + { + id: '0', + testResults: new Array(2).fill(null), + tester: { username: 'bot' }, + collectionJob: { + status: COLLECTION_JOB_STATUS.RUNNING, + testStatus: [ + { status: COLLECTION_JOB_STATUS.RUNNING }, + { status: COLLECTION_JOB_STATUS.COMPLETED }, + { status: COLLECTION_JOB_STATUS.QUEUED } + ] + } + }, + { + id: '1', + testResults: new Array(2).fill(null), + tester: { username: 'bot' }, + collectionJob: { + status: COLLECTION_JOB_STATUS.CANCELLED, + testStatus: [ + { status: COLLECTION_JOB_STATUS.CANCELLED }, + { status: COLLECTION_JOB_STATUS.COMPLETED }, + { status: COLLECTION_JOB_STATUS.CANCELLED } + ] + } + } + ]; + + const mocks = getMocks(testPlanRuns); + + const { getByText } = render( + <MockedProvider mocks={mocks} addTypename={false}> + <BotRunTestStatusList testPlanReportId="1" /> + </MockedProvider> + ); + + await waitFor(async () => { + // Wait for the component to update + // Imperfect but prevents needing to detect loading removal + await setTimeout(() => { + expect(getByText('1 Test Running')).toBeInTheDocument(); + expect(getByText('2 Tests Completed')).toBeInTheDocument(); + expect(getByText('1 Test Queued')).toBeInTheDocument(); + expect(getByText('2 Tests Cancelled')).toBeInTheDocument(); + }, 500); + }); +}); From 497ae8c9a2c5e3f530200486f68ccfe042a721fb Mon Sep 17 00:00:00 2001 From: "Mx. Corey Frang" <gnarf37@gmail.com> Date: Wed, 1 May 2024 11:59:53 -0400 Subject: [PATCH 14/14] correct test mock --- client/tests/BotRunTestStatusList.test.jsx | 1 - 1 file changed, 1 deletion(-) diff --git a/client/tests/BotRunTestStatusList.test.jsx b/client/tests/BotRunTestStatusList.test.jsx index dfb428703..2aec3b4c8 100644 --- a/client/tests/BotRunTestStatusList.test.jsx +++ b/client/tests/BotRunTestStatusList.test.jsx @@ -63,7 +63,6 @@ test('correctly ignores test results from a human-submitted test plan run', asyn collectionJob: { status: COLLECTION_JOB_STATUS.COMPLETED, testStatus: [ - { status: COLLECTION_JOB_STATUS.COMPLETED }, { status: COLLECTION_JOB_STATUS.COMPLETED }, { status: COLLECTION_JOB_STATUS.COMPLETED } ]