From d391b3bf23d477796769d1f43af0d86d19f9704e Mon Sep 17 00:00:00 2001 From: Howard Edwards Date: Mon, 18 Mar 2024 14:19:46 -0500 Subject: [PATCH 1/9] Update processCopiedReports to account for when commands have been added/removed between versions --- server/graphql-schema.js | 1 + .../resolvers/helpers/processCopiedReports.js | 636 ++++++++++-------- server/tests/integration/graphql.test.js | 3 +- server/util/aria.js | 11 +- 4 files changed, 365 insertions(+), 286 deletions(-) diff --git a/server/graphql-schema.js b/server/graphql-schema.js index 90f10dfa4..96ab28b15 100644 --- a/server/graphql-schema.js +++ b/server/graphql-schema.js @@ -600,6 +600,7 @@ const graphqlSchema = gql` A human-readable version of the assertion. """ text: String! + rawAssertionId: String } """ diff --git a/server/resolvers/helpers/processCopiedReports.js b/server/resolvers/helpers/processCopiedReports.js index bec9cd544..59f86d906 100644 --- a/server/resolvers/helpers/processCopiedReports.js +++ b/server/resolvers/helpers/processCopiedReports.js @@ -6,22 +6,249 @@ const { getOrCreateTestPlanReport, updateTestPlanReportById } = require('../../models/services/TestPlanReportService'); -const { - createTestResultId, - createScenarioResultId, - createAssertionResultId -} = require('../../services/PopulatedData/locationOfDataId'); const { hashTest } = require('../../util/aria'); const { createTestPlanRun, updateTestPlanRunById } = require('../../models/services/TestPlanRunService'); +const createTestResultSkeleton = require('../TestPlanRunOperations/createTestResultSkeleton'); const populateData = require('../../services/PopulatedData/populateData'); const runnableTestsResolver = require('../TestPlanReport/runnableTestsResolver'); const conflictsResolver = require('../TestPlanReport/conflictsResolver'); const finalizedTestResultsResolver = require('../TestPlanReport/finalizedTestResultsResolver'); const { getMetrics } = require('shared'); +/** + * Returns lists of known scenario and command ids for a test plan version's test + * @param testPlanVersionTest + * @returns {[string[],string[]]} + */ +const getKnownScenariosAndCommandIdsForTest = testPlanVersionTest => { + const knownAssertionIdsForTest = []; + const knownScenarioIdsForTest = []; + + for (const testPlanVersionAssertion of testPlanVersionTest.assertions) { + const { text, rawAssertionId } = testPlanVersionAssertion; + // `rawAssertionId` in v2 test format, otherwise `text` in v1 test format + knownAssertionIdsForTest.push(rawAssertionId || text); + } + + for (const testPlanVersionScenario of testPlanVersionTest.scenarios) { + const { commandIds, settings } = testPlanVersionScenario; + let scenarioId = commandIds.join('-'); + + // settings may not exist if v1 test format + if (settings) scenarioId = `${scenarioId}_${settings}`; + knownScenarioIdsForTest.push(scenarioId); + } + + return [knownScenarioIdsForTest, knownAssertionIdsForTest]; +}; + +/** + * Determine which tests between an 'old' and 'new' TestPlanVersion is the same between both + * @param oldTestPlanVersion + * @param newTestPlanVersion + * @returns {{}} + */ +const getKeptTestsByTestHash = (oldTestPlanVersion, newTestPlanVersion) => { + const updateOptions = { forUpdateCompare: true }; + const keptTestsByTestHash = {}; + + for (const newTestPlanVersionTest of newTestPlanVersion.tests) { + const newTestPlanVersionTestHash = hashTest( + { + ...newTestPlanVersionTest, + rowNumber: String(newTestPlanVersionTest.rowNumber) + }, + updateOptions + ); + + // Move to the next loop if the test hash is already being tracked + if (keptTestsByTestHash[newTestPlanVersionTestHash]) continue; + + for (const oldTestPlanVersionTest of oldTestPlanVersion.tests) { + const oldTestPlanVersionTestHash = hashTest( + { + ...oldTestPlanVersionTest, + rowNumber: String(oldTestPlanVersionTest.rowNumber) + }, + updateOptions + ); + + // Move to the next loop if the hashes don't match + if (newTestPlanVersionTestHash !== oldTestPlanVersionTestHash) + continue; + + // For each command and/or assertion under the test, will use to check if there is a + // matching command and/or assertion. If there is a match, preserve the previous result, + // otherwise mark the test as not being complete + const [knownScenarioIdsForOldTest, knownAssertionIdsForOldTest] = + getKnownScenariosAndCommandIdsForTest(oldTestPlanVersionTest); + const [knownScenarioIdsForNewTest, knownAssertionIdsForNewTest] = + getKnownScenariosAndCommandIdsForTest(newTestPlanVersionTest); + + keptTestsByTestHash[newTestPlanVersionTestHash] = { + newTestPlanVersionTestId: newTestPlanVersionTest.id, + oldTestPlanVersionTestId: oldTestPlanVersionTest.id, + knownScenarioIdsForOldTest, + knownAssertionIdsForOldTest, + knownScenarioIdsForNewTest, + knownAssertionIdsForNewTest + }; + } + } + + return keptTestsByTestHash; +}; + +/** + * Determine which old version's test result should be preserved and which new result it connects to + * @param testResults + * @param keptTestsByTestHash + * @returns {{}} + */ +const getKeptTestResultsByTestId = (testResults, keptTestsByTestHash) => { + const keptTestResultsByTestId = {}; + for (const testResult of testResults) { + // Check if the testId referenced also matches the hash on any in the + // keptTestsByTestHash + Object.keys(keptTestsByTestHash).forEach(key => { + const { + newTestPlanVersionTestId, + oldTestPlanVersionTestId, + knownAssertionIdsForOldTest, + knownScenarioIdsForOldTest, + knownAssertionIdsForNewTest, + knownScenarioIdsForNewTest + } = keptTestsByTestHash[key]; + + if (oldTestPlanVersionTestId === testResult.testId) { + // Then this data should be preserved + keptTestResultsByTestId[newTestPlanVersionTestId] = { + ...testResult, + knownAssertionIdsForOldTest, + knownScenarioIdsForOldTest, + knownAssertionIdsForNewTest, + knownScenarioIdsForNewTest + }; + } else { + // TODO: Return information on which tests cannot be preserved + } + }); + } + + return keptTestResultsByTestId; +}; + +/** + * Updates metrics and markedFinalAt status for newly created TestPlanReports after copying process + * @param oldTestPlanReport + * @param newTestPlanReport + * @param testPlanRun + * @param testResults + * @param context + * @param transaction + * @returns {Promise} + */ +const updateMetricsAndMarkedFinalAtForTestPlanReport = async ({ + oldTestPlanReport, + newTestPlanReport, + testPlanRun, + testResults, + context, + transaction +}) => { + await updateTestPlanRunById({ + id: testPlanRun.id, + values: { testResults }, + transaction + }); + + // Update metrics for TestPlanReport + const { testPlanReport: populatedTestPlanReport } = await populateData( + { testPlanReportId: newTestPlanReport.id }, + { transaction } + ); + + const runnableTests = runnableTestsResolver( + populatedTestPlanReport, + null, + context + ); + let updateParams = {}; + + // Mark the report as final if previously was for TestPlanVersion being deprecated; may still be + // nullified if finalized test results aren't equal to the amount known number of possible + // runnable tests, because no tests should be skipped. Would mean it CANNOT be final. + if (oldTestPlanReport.markedFinalAt) + updateParams = { markedFinalAt: new Date() }; + + // Calculate the metrics (happens if updating to DRAFT) + const conflicts = await conflictsResolver( + populatedTestPlanReport, + null, + context + ); + + if (conflicts.length > 0) { + // Then no chance to have finalized reports, and means it hasn't been + // marked as final yet + updateParams = { + ...updateParams, + markedFinalAt: null, + metrics: { + ...populatedTestPlanReport.metrics, + conflictsCount: conflicts.length + } + }; + } else { + const finalizedTestResults = await finalizedTestResultsResolver( + populatedTestPlanReport, + null, + context + ); + + if (!finalizedTestResults || !finalizedTestResults.length) { + updateParams = { + ...updateParams, + markedFinalAt: null, + metrics: { + ...populatedTestPlanReport.metrics + } + }; + } else { + const metrics = getMetrics({ + testPlanReport: { + ...populatedTestPlanReport, + finalizedTestResults, + runnableTests + } + }); + + updateParams = { + ...updateParams, + // means test results have now been 'skipped' during the update process so these + // cannot be finalized and must be updated in the test queue + markedFinalAt: + finalizedTestResults.length < runnableTests.length + ? null + : updateParams.markedFinalAt, + metrics: { + ...populatedTestPlanReport.metrics, + ...metrics + } + }; + } + } + + await updateTestPlanReportById({ + id: populatedTestPlanReport.id, + values: updateParams, + transaction + }); +}; + const processCopiedReports = async ({ oldTestPlanVersionId, newTestPlanVersionId, @@ -74,300 +301,149 @@ const processCopiedReports = async ({ // If there is an earlier version that for this phase and that version has some test plan runs // in the test queue, this will run the process for updating existing test plan versions for the // test plan version and preserving data for tests that have not changed. - const forUpdateCompare = true; - for (const oldTestPlanReport of oldTestPlanReports) { - // Verify the combination does not exist + // Verify an existing combination for the TestPlanVersion being updated to, does not already + // exist if ( - !newTestPlanReports.some( + newTestPlanReports.some( ({ atId, browserId }) => atId === oldTestPlanReport.atId && browserId === oldTestPlanReport.browserId ) ) { - // Then this combination needs to be considered if the tests are not different - // between versions - let keptTestIds = {}; - for (const newTestPlanVersionTest of newTestPlanVersion.tests) { - // Found odd instances of rowNumber being an int instead of being how it - // currently is; imported as a string - // Ensuring proper hashes are done here - const newTestPlanVersionTestHash = hashTest( - { - ...newTestPlanVersionTest, - rowNumber: String(newTestPlanVersionTest.rowNumber) - }, - { forUpdateCompare } - ); + continue; + } - if (keptTestIds[newTestPlanVersionTestHash]) continue; - - for (const oldTestPlanVersionTest of oldTestPlanVersion.tests) { - const oldTestPlanVersionTestHash = hashTest( - { - ...oldTestPlanVersionTest, - rowNumber: String(oldTestPlanVersionTest.rowNumber) - }, - { forUpdateCompare } - ); - - // For each assertion under the test, check if there is a matching - // assertion. If the assertion matches, preserve the previous result, - // otherwise mark the test as not being complete - const knownAssertionIdsForOldTest = []; - for (const oldTestPlanVersionAssertion of oldTestPlanVersionTest.assertions) { - if (oldTestPlanVersionAssertion.rawAssertionId) - knownAssertionIdsForOldTest.push( - oldTestPlanVersionAssertion.rawAssertionId - ); - } + // Then the combination needs to be considered if the tests are not different + // between versions + const keptTestsByTestHash = getKeptTestsByTestHash( + oldTestPlanVersion, + newTestPlanVersion + ); + + for (const oldTestPlanRun of oldTestPlanReport.testPlanRuns) { + // Track which old test results need to be preserved + const keptTestResultsByTestId = getKeptTestResultsByTestId( + oldTestPlanRun.testResults, + keptTestsByTestHash + ); + + if (!Object.keys(keptTestResultsByTestId).length) continue; + + // Create (or get) the new test plan report the results will be copied to + const [newTestPlanReport] = await getOrCreateTestPlanReport({ + where: { + testPlanVersionId: newTestPlanVersionId, + atId: oldTestPlanReport.atId, + browserId: oldTestPlanReport.browserId + }, + transaction + }); + newTestPlanReportIds.push(newTestPlanReport.id); + + // Create the new test plan run for a previous assigned tester so old results can be + // copied to it + const newTestPlanRun = await createTestPlanRun({ + values: { + testerUserId: oldTestPlanRun.testerUserId, + testPlanReportId: newTestPlanReport.id + }, + transaction + }); + const newTestResults = []; + + for (const testResultToSaveTestId of Object.keys( + keptTestResultsByTestId + )) { + const oldTestResult = + keptTestResultsByTestId[testResultToSaveTestId]; + const { + knownAssertionIdsForOldTest, + knownScenarioIdsForOldTest, + knownAssertionIdsForNewTest, + knownScenarioIdsForNewTest + } = oldTestResult; + + const { test } = await populateData( + { testId: testResultToSaveTestId }, + { transaction } + ); - if ( - newTestPlanVersionTestHash === - oldTestPlanVersionTestHash - ) { - if (!keptTestIds[newTestPlanVersionTestHash]) - keptTestIds[newTestPlanVersionTestHash] = { - newTestPlanVersionTestId: - newTestPlanVersionTest.id, - oldTestPlanVersionTestId: - oldTestPlanVersionTest.id, - knownAssertionIdsForOldTest - }; + // Re-run createTestResultSkeleton to avoid unexpected scenario index matching issues when saving + // future results; override newly generated test results with old results if exists + let newTestResult = createTestResultSkeleton({ + test, + testPlanRun: newTestPlanRun, + testPlanReport: newTestPlanReport, + atVersionId: oldTestResult.atVersionId, + browserVersionId: oldTestResult.browserVersionId + }); + newTestResult.completedAt = oldTestResult.completedAt; + + const scenarioResultsByScenarioIds = {}; + knownScenarioIdsForOldTest.forEach((id, index) => { + scenarioResultsByScenarioIds[id] = + oldTestResult.scenarioResults[index]; + }); + + // Preserve output and unexpectedBehaviors for each scenario if matching old result + for (let [ + scenarioIndex, + eachScenarioResult + ] of newTestResult.scenarioResults.entries()) { + const rawScenarioId = + knownScenarioIdsForNewTest[scenarioIndex]; + + // Unknown combination of command + settings when compared with last version + const oldScenarioResult = + scenarioResultsByScenarioIds[rawScenarioId]; + if (!oldScenarioResult) { + newTestResult.completedAt = null; + continue; } - } - } - - for (const oldTestPlanRun of oldTestPlanReport.testPlanRuns) { - const newTestResultsToSaveByTestId = {}; - for (const oldTestResult of oldTestPlanRun.testResults) { - // Check if the testId referenced also matches the hash on any in the - // keptTestIds - Object.keys(keptTestIds).forEach(key => { - const { - newTestPlanVersionTestId, - oldTestPlanVersionTestId, - knownAssertionIdsForOldTest - } = keptTestIds[key]; - - if (oldTestPlanVersionTestId === oldTestResult.testId) { - // Then this data should be preserved - newTestResultsToSaveByTestId[ - newTestPlanVersionTestId - ] = { - ...oldTestResult, - knownAssertionIdsForOldTest - }; - } else { - // TODO: Return information on which tests cannot be preserved - } - }); - } - if (Object.keys(newTestResultsToSaveByTestId).length) { - const [newTestPlanReport] = await getOrCreateTestPlanReport( - { - where: { - testPlanVersionId: newTestPlanVersionId, - atId: oldTestPlanReport.atId, - browserId: oldTestPlanReport.browserId - }, - transaction - } - ); + eachScenarioResult.output = oldScenarioResult.output; + eachScenarioResult.unexpectedBehaviors = + oldScenarioResult.unexpectedBehaviors; - newTestPlanReportIds.push(newTestPlanReport.id); - - const newTestPlanRun = await createTestPlanRun({ - values: { - testerUserId: oldTestPlanRun.testerUserId, - testPlanReportId: newTestPlanReport.id - }, - transaction + const assertionResultsByAssertionIds = {}; + knownAssertionIdsForOldTest.forEach((id, index) => { + assertionResultsByAssertionIds[id] = + oldScenarioResult.assertionResults[index]; }); - const newTestResults = []; - for (const testResultToSaveTestId of Object.keys( - newTestResultsToSaveByTestId - )) { - const foundKeptNewTest = newTestPlanVersion.tests.find( - test => test.id === testResultToSaveTestId - ); - - let newTestResult = - newTestResultsToSaveByTestId[ - testResultToSaveTestId - ]; - - const knownAssertionIdsForOldTest = [ - ...newTestResult.knownAssertionIdsForOldTest - ]; - delete newTestResult.knownAssertionIdsForOldTest; - - // Updating testResult id references - const newTestResultId = createTestResultId( - newTestPlanRun.id, - testResultToSaveTestId - ); - - newTestResult.testId = testResultToSaveTestId; - newTestResult.id = newTestResultId; - - // The hash confirms the scenario (commands) arrays should be in the same - // order, and regenerate the test result related ids for the carried - // over data - for (let [ - scenarioIndex, - eachScenarioResult - ] of newTestResult.scenarioResults.entries()) { - eachScenarioResult.scenarioId = - foundKeptNewTest.scenarios.filter( - scenario => - scenario.atId === oldTestPlanReport.atId - )[scenarioIndex].id; - - // Update eachScenarioResult.id - const scenarioResultId = createScenarioResultId( - newTestResultId, - eachScenarioResult.scenarioId - ); - eachScenarioResult.id = scenarioResultId; - - for (let [ - assertionIndex, - eachAssertionResult - ] of eachScenarioResult.assertionResults.entries()) { - eachAssertionResult.assertionId = - foundKeptNewTest.assertions[ - assertionIndex - ].id; - - // Update eachAssertionResult.id - eachAssertionResult.id = - createAssertionResultId( - scenarioResultId, - eachAssertionResult.assertionId - ); - - // Checks if rawAssertionId is persisted across TestPlanVersions - // Nullify 'passed' and mark the test as not completed if it isn't - if ( - knownAssertionIdsForOldTest.length === - foundKeptNewTest.assertions.length && - knownAssertionIdsForOldTest[ - assertionIndex - ] !== - foundKeptNewTest.assertions[ - assertionIndex - ].rawAssertionId - ) { - eachAssertionResult.passed = null; - newTestResult.completedAt = null; - } - } + // Preserve passed status for each assertion if matching old result + for (let [ + assertionIndex, + eachAssertionResult + ] of eachScenarioResult.assertionResults.entries()) { + const rawAssertionId = + knownAssertionIdsForNewTest[assertionIndex]; + + // Unknown assertion when compared with last version + const oldAssertionResult = + assertionResultsByAssertionIds[rawAssertionId]; + if (!oldAssertionResult) { + newTestResult.completedAt = null; + continue; } - newTestResults.push(newTestResult); - } - - // Update TestPlanRun test results to be used in metrics evaluation - // afterward - await updateTestPlanRunById({ - id: newTestPlanRun.id, - values: { testResults: newTestResults }, - transaction - }); - - // Update metrics for TestPlanReport - const { testPlanReport: populatedTestPlanReport } = - await populateData( - { testPlanReportId: newTestPlanReport.id }, - { transaction } - ); - - const runnableTests = runnableTestsResolver( - populatedTestPlanReport, - null, - context - ); - let updateParams = {}; - - // Mark the report as final if previously was for TestPlanVersion being deprecated; may still be - // nullified if finalized test results aren't equal to the amount known number of possible - // runnable tests, because no tests should be skipped. Would mean it CANNOT be final. - if (oldTestPlanReport.markedFinalAt) - updateParams = { markedFinalAt: new Date() }; - - // Calculate the metrics (happens if updating to DRAFT) - const conflicts = await conflictsResolver( - populatedTestPlanReport, - null, - context - ); - - if (conflicts.length > 0) { - // Then no chance to have finalized reports, and means it hasn't been - // marked as final yet - updateParams = { - ...updateParams, - markedFinalAt: null, - metrics: { - ...populatedTestPlanReport.metrics, - conflictsCount: conflicts.length - } - }; - } else { - const finalizedTestResults = - await finalizedTestResultsResolver( - populatedTestPlanReport, - null, - context - ); - - if ( - !finalizedTestResults || - !finalizedTestResults.length - ) { - updateParams = { - ...updateParams, - markedFinalAt: null, - metrics: { - ...populatedTestPlanReport.metrics - } - }; - } else { - const metrics = getMetrics({ - testPlanReport: { - ...populatedTestPlanReport, - finalizedTestResults, - runnableTests - } - }); - - updateParams = { - ...updateParams, - // means test results have now been 'skipped' during the update process so these - // cannot be finalized and must be updated in the test queue - markedFinalAt: - finalizedTestResults.length < - runnableTests.length - ? null - : updateParams.markedFinalAt, - metrics: { - ...populatedTestPlanReport.metrics, - ...metrics - } - }; - } + eachAssertionResult.passed = oldAssertionResult.passed; } - - await updateTestPlanReportById({ - id: populatedTestPlanReport.id, - values: updateParams, - transaction - }); } + + newTestResults.push(newTestResult); } + + // Run updated metrics calculations for new TestPlanRun test results to be used in metrics calculations + await updateMetricsAndMarkedFinalAtForTestPlanReport({ + oldTestPlanReport, + newTestPlanReport, + testPlanRun: newTestPlanRun, + testResults: newTestResults, + context, + transaction + }); } } diff --git a/server/tests/integration/graphql.test.js b/server/tests/integration/graphql.test.js index b89ad2996..f798a6762 100644 --- a/server/tests/integration/graphql.test.js +++ b/server/tests/integration/graphql.test.js @@ -163,7 +163,8 @@ describe('graphql', () => { // which is mocked in other tests. ['Mutation', 'scheduleCollectionJob'], ['Mutation', 'restartCollectionJob'], - ['CollectionJobOperations', 'retryCanceledCollections'] + ['CollectionJobOperations', 'retryCanceledCollections'], + ['Assertion', 'rawAssertionId'] ]; ({ typeAwareQuery, diff --git a/server/util/aria.js b/server/util/aria.js index e08e4287b..cae6adb10 100644 --- a/server/util/aria.js +++ b/server/util/aria.js @@ -24,14 +24,15 @@ const testWithModifiedAttributes = (test, { forUpdateCompare }) => { // Changed text in renderableContent.assertions[].assertion(Statement|Phrase) shouldn't // matter during comparison of results propertiesToOmit.push('renderableContent.assertions'); - - // The collection of assertions won't matter during an update comparison; a per-assertion - // check will that is handled separately in processCopiedReports.js propertiesToOmit.push('assertions'); + // The collection of scenarios (commands) won't matter during an update comparison; + // a per-command check will that is handled separately in processCopiedReports.js + propertiesToOmit.push('renderableContent.commands'); + propertiesToOmit.push('scenarios'); + return { - ...omit(test, propertiesToOmit), - scenarios: test.scenarios.map(scenario => omit(scenario, ['id'])) + ...omit(test, propertiesToOmit) }; } else { return { From d096e19a0c82de8e9b920320c338d79bb9187903 Mon Sep 17 00:00:00 2001 From: Howard Edwards Date: Tue, 19 Mar 2024 09:37:44 -0500 Subject: [PATCH 2/9] Revert unnecessary changes to graphql schema --- server/graphql-schema.js | 1 - server/tests/integration/graphql.test.js | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/server/graphql-schema.js b/server/graphql-schema.js index 96ab28b15..90f10dfa4 100644 --- a/server/graphql-schema.js +++ b/server/graphql-schema.js @@ -600,7 +600,6 @@ const graphqlSchema = gql` A human-readable version of the assertion. """ text: String! - rawAssertionId: String } """ diff --git a/server/tests/integration/graphql.test.js b/server/tests/integration/graphql.test.js index f798a6762..b89ad2996 100644 --- a/server/tests/integration/graphql.test.js +++ b/server/tests/integration/graphql.test.js @@ -163,8 +163,7 @@ describe('graphql', () => { // which is mocked in other tests. ['Mutation', 'scheduleCollectionJob'], ['Mutation', 'restartCollectionJob'], - ['CollectionJobOperations', 'retryCanceledCollections'], - ['Assertion', 'rawAssertionId'] + ['CollectionJobOperations', 'retryCanceledCollections'] ]; ({ typeAwareQuery, From 3a6e9d43b99239c6908b722850288d5d0b993b84 Mon Sep 17 00:00:00 2001 From: Howard Edwards Date: Wed, 20 Mar 2024 18:16:54 -0500 Subject: [PATCH 3/9] Account for v1 format when updating results --- server/resolvers/helpers/processCopiedReports.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/resolvers/helpers/processCopiedReports.js b/server/resolvers/helpers/processCopiedReports.js index bcf86b3fb..abeb62eb4 100644 --- a/server/resolvers/helpers/processCopiedReports.js +++ b/server/resolvers/helpers/processCopiedReports.js @@ -34,8 +34,9 @@ const getKnownScenariosAndCommandIdsForTest = testPlanVersionTest => { } for (const testPlanVersionScenario of testPlanVersionTest.scenarios) { - const { commandIds, settings } = testPlanVersionScenario; + const { commandIds, atId, settings } = testPlanVersionScenario; let scenarioId = commandIds.join('-'); + scenarioId = `${scenarioId}_${atId}`; // settings may not exist if v1 test format if (settings) scenarioId = `${scenarioId}_${settings}`; From 621de5d792d309fe703ca441b31f60744d1949be Mon Sep 17 00:00:00 2001 From: Howard Edwards Date: Thu, 21 Mar 2024 08:18:29 -0500 Subject: [PATCH 4/9] Update hashing functionality to ignore certain properties to facilitate all the requirements of copying results --- server/util/aria.js | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/server/util/aria.js b/server/util/aria.js index cae6adb10..7e9d7b799 100644 --- a/server/util/aria.js +++ b/server/util/aria.js @@ -17,10 +17,22 @@ const testWithModifiedAttributes = (test, { forUpdateCompare }) => { 'viewers' ]; - // During comparison for phase update, we only need to make sure the assertionId hasn't - // changed so the potentially copied result isn't affected by the assertionStatement or - // assertionPhrase being changed + // During comparison for phase update, we need to make sure the assertionId and + // commandIds hasn't changed so the potentially copied result isn't affected by + // the assertionStatement, assertionPhrase, settings or instructions content + // being changed if (forUpdateCompare) { + // Don't factor in settings and instructions changes during update + propertiesToOmit.push('renderableContent.target.at.settings'); + propertiesToOmit.push('renderableContent.instructions'); + // for v1 format since structure is: + // { ..., renderableContent: { 1: ..., 2: ... }, ... } + for (let key in test.renderableContent) { + test.renderableContent[key] = omit(test.renderableContent[key], [ + 'instructions' + ]); + } + // Changed text in renderableContent.assertions[].assertion(Statement|Phrase) shouldn't // matter during comparison of results propertiesToOmit.push('renderableContent.assertions'); From e6c484befafbfa653e589f11b6fa6f10e60039ef Mon Sep 17 00:00:00 2001 From: Howard Edwards Date: Thu, 21 Mar 2024 16:13:06 -0500 Subject: [PATCH 5/9] Fix API call --- server/resolvers/helpers/processCopiedReports.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/resolvers/helpers/processCopiedReports.js b/server/resolvers/helpers/processCopiedReports.js index 900b3b698..3ec615146 100644 --- a/server/resolvers/helpers/processCopiedReports.js +++ b/server/resolvers/helpers/processCopiedReports.js @@ -368,7 +368,7 @@ const processCopiedReports = async ({ const { test } = await populateData( { testId: testResultToSaveTestId }, - { transaction } + { context } ); // Re-run createTestResultSkeleton to avoid unexpected scenario index matching issues when saving From 7f707ad61acbdbfaaf7938f078ebdc8fc2f1c5b3 Mon Sep 17 00:00:00 2001 From: Howard Edwards Date: Mon, 25 Mar 2024 09:13:35 -0500 Subject: [PATCH 6/9] Update tests --- .../tests/integration/dataManagement.test.js | 123 ++++++++++++++++++ server/util/aria.js | 3 + 2 files changed, 126 insertions(+) diff --git a/server/tests/integration/dataManagement.test.js b/server/tests/integration/dataManagement.test.js index d1b38e349..0f3d9481a 100644 --- a/server/tests/integration/dataManagement.test.js +++ b/server/tests/integration/dataManagement.test.js @@ -739,4 +739,127 @@ describe('data management', () => { ).toEqual(oldCommandButtonVersionMarkedFinalReportsCount - 1); }); }); + + it('preserves results for all results where commands or assertions are unchanged', async () => { + await dbCleaner(async transaction => { + function countCollectedAssertionResults(run) { + return run.testResults.reduce((trSum, tr) => { + return ( + trSum + + tr.scenarioResults.reduce((srSum, sr) => { + // Check if assertion has been collected; null + // if not yet collected + return ( + srSum + + sr.assertionResults.filter( + ({ passed }) => passed != null + ).length + ); + }, 0) + ); + }, 0); + } + + const testPlanVersions = await testPlanVersionsQuery({ + transaction + }); + + // Process counts for old version + const oldCommandButtonVersion = + testPlanVersions.testPlanVersions.find( + e => + e.testPlan.directory === 'command-button' && + e.phase === 'DRAFT' && + e.metadata.testFormatVersion === 2 + ); + const oldJAWSReport = oldCommandButtonVersion.testPlanReports.find( + ({ at }) => at.id == 1 + ); + const oldNVDAReport = oldCommandButtonVersion.testPlanReports.find( + ({ at }) => at.id == 2 + ); + const oldVOReport = oldCommandButtonVersion.testPlanReports.find( + ({ at }) => at.id == 3 + ); + + // They're all marked as final so only one run matters for this test + const [oldJAWSRun] = oldJAWSReport.draftTestPlanRuns; + const [oldNVDARun] = oldNVDAReport.draftTestPlanRuns; + const [oldVORun] = oldVOReport.draftTestPlanRuns; + + const oldJAWSAssertionsCollectedCount = + countCollectedAssertionResults(oldJAWSRun); + const oldNVDAAssertionsCollectedCount = + countCollectedAssertionResults(oldNVDARun); + const oldVOAssertionsCollectedCount = + countCollectedAssertionResults(oldVORun); + + // Process counts for new version + const [newCommandButtonVersion] = + testPlanVersions.testPlanVersions.filter( + e => + e.testPlan.directory === 'command-button' && + e.phase === 'RD' && + e.metadata.testFormatVersion === 2 + ); + + const { testPlanVersion: newCommandButtonVersionInDraft } = + await updateVersionToPhaseQuery( + newCommandButtonVersion.id, + 'DRAFT', + { + testPlanVersionDataToIncludeId: + oldCommandButtonVersion.id, + transaction + } + ); + + const newJAWSReport = + newCommandButtonVersionInDraft.updatePhase.testPlanVersion.testPlanReports.find( + ({ at }) => at.id == 1 + ); + const newNVDAReport = + newCommandButtonVersionInDraft.updatePhase.testPlanVersion.testPlanReports.find( + ({ at }) => at.id == 2 + ); + const newVOReport = + newCommandButtonVersionInDraft.updatePhase.testPlanVersion.testPlanReports.find( + ({ at }) => at.id == 3 + ); + + // They're all marked as final so only one run matters for this test + const [newJAWSRun] = newJAWSReport.draftTestPlanRuns; + const [newNVDARun] = newNVDAReport.draftTestPlanRuns; + const [newVORun] = newVOReport.draftTestPlanRuns; + + const newJAWSAssertionsCollectedCount = + countCollectedAssertionResults(newJAWSRun); + const newNVDAAssertionsCollectedCount = + countCollectedAssertionResults(newNVDARun); + const newVOAssertionsCollectedCount = + countCollectedAssertionResults(newVORun); + + // The difference between them is that there have been updated settings for VoiceOver tests; + // 2 were switched from 'quickNavOn' to 'singleQuickKeyNavOn' which means the tracked command + // has been changed. + // + // Based on https://github.com/w3c/aria-at/compare/d9a19f8...565a87b#diff-4e3dcd0a202f268ebec2316344f136c3a83d6e03b3f726775cb46c57322ff3a0, + // only 'navForwardsToButton' and 'navBackToButton' tests were affected. The individual tests for + // 'reqInfoAboutButton' should still match. + // + // This means only 1 test plan report should be affected, for VoiceOver for 2 tests, + // for 1 command, which carries 2 assertions. So 2 tests * 2 assertions = 4 assertions + // have been affected. + // The JAWS and NVDA reports should be unaffected. + expect(newJAWSAssertionsCollectedCount).toEqual( + oldJAWSAssertionsCollectedCount + ); + expect(newNVDAAssertionsCollectedCount).toEqual( + oldNVDAAssertionsCollectedCount + ); + expect(newVOAssertionsCollectedCount).toEqual( + oldVOAssertionsCollectedCount - 4 + ); + }); + }); }); diff --git a/server/util/aria.js b/server/util/aria.js index 7e9d7b799..930ac3847 100644 --- a/server/util/aria.js +++ b/server/util/aria.js @@ -23,6 +23,9 @@ const testWithModifiedAttributes = (test, { forUpdateCompare }) => { // being changed if (forUpdateCompare) { // Don't factor in settings and instructions changes during update + // eligibility in the case where ONLY the instructions has changed, + // so that the updated instructions or settings are shown when the + // copy process happens propertiesToOmit.push('renderableContent.target.at.settings'); propertiesToOmit.push('renderableContent.instructions'); // for v1 format since structure is: From 3d181461cd8e4f429c06c67689659059731fbd50 Mon Sep 17 00:00:00 2001 From: Howard Edwards Date: Tue, 26 Mar 2024 09:35:28 -0500 Subject: [PATCH 7/9] Fix test tile --- server/tests/integration/dataManagement.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/tests/integration/dataManagement.test.js b/server/tests/integration/dataManagement.test.js index 0f3d9481a..84323a80e 100644 --- a/server/tests/integration/dataManagement.test.js +++ b/server/tests/integration/dataManagement.test.js @@ -740,7 +740,7 @@ describe('data management', () => { }); }); - it('preserves results for all results where commands or assertions are unchanged', async () => { + it('preserves results for copied report combinations where commands or assertions are unchanged', async () => { await dbCleaner(async transaction => { function countCollectedAssertionResults(run) { return run.testResults.reduce((trSum, tr) => { From 2d6b014e2eecf37fb1816e67625f31c0351ff0cf Mon Sep 17 00:00:00 2001 From: Howard Edwards Date: Wed, 3 Apr 2024 15:41:30 -0500 Subject: [PATCH 8/9] Unmarks newly copied test plan reports as final during results copy process (#974) * Unmarks test plan reports as being final if copied from older reports * Update tests with new logic; noticed results being lost when settings updated for a test * Delete server/migrations/20240320205951-regenerateHashes.js because it won't be needed depending on when this change goes to production * Fix propertiesToOmit --- .../resolvers/helpers/processCopiedReports.js | 22 ++--- .../tests/integration/dataManagement.test.js | 85 +++++++++++++++---- 2 files changed, 74 insertions(+), 33 deletions(-) diff --git a/server/resolvers/helpers/processCopiedReports.js b/server/resolvers/helpers/processCopiedReports.js index 3ec615146..da3244d7c 100644 --- a/server/resolvers/helpers/processCopiedReports.js +++ b/server/resolvers/helpers/processCopiedReports.js @@ -144,7 +144,6 @@ const getKeptTestResultsByTestId = (testResults, keptTestsByTestHash) => { /** * Updates metrics and markedFinalAt status for newly created TestPlanReports after copying process - * @param oldTestPlanReport * @param newTestPlanReport * @param testPlanRun * @param testResults @@ -153,7 +152,6 @@ const getKeptTestResultsByTestId = (testResults, keptTestsByTestHash) => { * @returns {Promise} */ const updateMetricsAndMarkedFinalAtForTestPlanReport = async ({ - oldTestPlanReport, newTestPlanReport, testPlanRun, testResults, @@ -177,13 +175,12 @@ const updateMetricsAndMarkedFinalAtForTestPlanReport = async ({ null, context ); - let updateParams = {}; - // Mark the report as final if previously was for TestPlanVersion being deprecated; may still be - // nullified if finalized test results aren't equal to the amount known number of possible - // runnable tests, because no tests should be skipped. Would mean it CANNOT be final. - if (oldTestPlanReport.markedFinalAt) - updateParams = { markedFinalAt: new Date() }; + // Even if oldTestPlanReport.markedFinalAt exists, it should be nullified + // when copied over because it should be on the test admin(s) to ensure + // the copied data isn't incorrect in certain instances or needs to be + // updated before finalizing again + let updateParams = { markedFinalAt: null }; // Calculate the metrics (happens if updating to DRAFT) const conflicts = await conflictsResolver( @@ -197,7 +194,6 @@ const updateMetricsAndMarkedFinalAtForTestPlanReport = async ({ // marked as final yet updateParams = { ...updateParams, - markedFinalAt: null, metrics: { ...populatedTestPlanReport.metrics, conflictsCount: conflicts.length @@ -213,7 +209,6 @@ const updateMetricsAndMarkedFinalAtForTestPlanReport = async ({ if (!finalizedTestResults || !finalizedTestResults.length) { updateParams = { ...updateParams, - markedFinalAt: null, metrics: { ...populatedTestPlanReport.metrics } @@ -229,12 +224,6 @@ const updateMetricsAndMarkedFinalAtForTestPlanReport = async ({ updateParams = { ...updateParams, - // means test results have now been 'skipped' during the update process so these - // cannot be finalized and must be updated in the test queue - markedFinalAt: - finalizedTestResults.length < runnableTests.length - ? null - : updateParams.markedFinalAt, metrics: { ...populatedTestPlanReport.metrics, ...metrics @@ -439,7 +428,6 @@ const processCopiedReports = async ({ // Run updated metrics calculations for new TestPlanRun test results to be used in metrics calculations await updateMetricsAndMarkedFinalAtForTestPlanReport({ - oldTestPlanReport, newTestPlanReport, testPlanRun: newTestPlanRun, testResults: newTestResults, diff --git a/server/tests/integration/dataManagement.test.js b/server/tests/integration/dataManagement.test.js index 84323a80e..0e5cb5740 100644 --- a/server/tests/integration/dataManagement.test.js +++ b/server/tests/integration/dataManagement.test.js @@ -674,16 +674,29 @@ describe('data management', () => { }); }); - it('updates test plan version but removes marked as final for test plan report when new report has incomplete runs', async () => { + it('updates test plan version over another which had all test plan reports as final but removes final for all newly created test plan reports', async () => { await dbCleaner(async transaction => { - function markedFinalAtReduce(acc, curr) { - return acc + (curr.markedFinalAt ? 1 : 0); + function markedFinalAtFilter({ markedFinalAt }) { + return !!markedFinalAt; + } + + function completedResultsCount(testPlanReports, atId) { + return ( + testPlanReports + .find(({ at }) => at.id == atId) + // 0 because only 1 tester for each already marked final + // report matters during this test + .draftTestPlanRuns[0].testResults.filter( + ({ completedAt }) => !!completedAt + ).length + ); } const testPlanVersions = await testPlanVersionsQuery({ transaction }); + // Process counts for old test plan version const [oldCommandButtonVersion] = testPlanVersions.testPlanVersions.filter( e => @@ -691,13 +704,27 @@ describe('data management', () => { e.phase === 'DRAFT' && e.metadata.testFormatVersion === 2 ); - const oldCommandButtonVersionMarkedFinalReportsCount = - oldCommandButtonVersion.testPlanReports.reduce( - markedFinalAtReduce, - 0 + oldCommandButtonVersion.testPlanReports.filter( + markedFinalAtFilter + ).length; + const oldCommandButtonJawsCompletedTestResultsCount = + completedResultsCount( + oldCommandButtonVersion.testPlanReports, + 1 + ); + const oldCommandButtonNvdaCompletedTestResultsCount = + completedResultsCount( + oldCommandButtonVersion.testPlanReports, + 2 + ); + const oldCommandButtonSafariCompletedTestResultsCount = + completedResultsCount( + oldCommandButtonVersion.testPlanReports, + 3 ); + // Process counts for new test plan version const [newCommandButtonVersion] = testPlanVersions.testPlanVersions.filter( e => @@ -705,7 +732,6 @@ describe('data management', () => { e.phase === 'RD' && e.metadata.testFormatVersion === 2 ); - const { testPlanVersion: newCommandButtonVersionInDraft } = await updateVersionToPhaseQuery( newCommandButtonVersion.id, @@ -717,9 +743,26 @@ describe('data management', () => { } ); const newCommandButtonVersionInDraftMarkedFinalReportsCount = - newCommandButtonVersionInDraft.updatePhase.testPlanVersion.testPlanReports.reduce( - markedFinalAtReduce, - 0 + newCommandButtonVersionInDraft.updatePhase.testPlanVersion.testPlanReports.filter( + markedFinalAtFilter + ).length; + const newCommandButtonJawsCompletedTestResultsCount = + completedResultsCount( + newCommandButtonVersionInDraft.updatePhase.testPlanVersion + .testPlanReports, + 1 + ); + const newCommandButtonNvdaCompletedTestResultsCount = + completedResultsCount( + newCommandButtonVersionInDraft.updatePhase.testPlanVersion + .testPlanReports, + 2 + ); + const newCommandButtonSafariCompletedTestResultsCount = + completedResultsCount( + newCommandButtonVersionInDraft.updatePhase.testPlanVersion + .testPlanReports, + 3 ); // The difference between them is that there have been updated settings for VoiceOver tests; @@ -729,14 +772,24 @@ describe('data management', () => { // only 'navForwardsToButton' and 'navBackToButton' tests were affected. The individual tests for // 'reqInfoAboutButton' should still match // - // This means only 1 test plan report should be affected, for VoiceOver and now unmarked as final. - // The single JAWS and NVDA reports should be unaffected + // This means only 1 test plan report was affected results-wise, for VoiceOver, but they should all still be + // unmarked as final. The single JAWS and NVDA reports should have unaffected results but also unmarked as + // final. expect( - newCommandButtonVersionInDraftMarkedFinalReportsCount - ).toBeGreaterThan(1); + oldCommandButtonVersionMarkedFinalReportsCount + ).toBeGreaterThan(0); expect( newCommandButtonVersionInDraftMarkedFinalReportsCount - ).toEqual(oldCommandButtonVersionMarkedFinalReportsCount - 1); + ).toEqual(0); + expect(newCommandButtonJawsCompletedTestResultsCount).toEqual( + oldCommandButtonJawsCompletedTestResultsCount + ); + expect(newCommandButtonNvdaCompletedTestResultsCount).toEqual( + oldCommandButtonNvdaCompletedTestResultsCount + ); + expect(newCommandButtonSafariCompletedTestResultsCount).toEqual( + oldCommandButtonSafariCompletedTestResultsCount - 2 + ); }); }); From f0afc67ad6d09e3810c86aacee74610f8f7c36ad Mon Sep 17 00:00:00 2001 From: Howard Edwards Date: Mon, 8 Apr 2024 11:02:14 -0500 Subject: [PATCH 9/9] feat: Copy results from advanced phase (#976) * Copies results into new TestPlanReports for a version when transitioning to DRAFT from CANDIDATE or RECOMMENDED if applicable * Copies results into new TestPlanReports for a version when transitioning to CANDIDATE from RECOMMENDED if applicable --- .../AddTestToQueueWithConfirmation/index.jsx | 154 ++++++++++++++---- .../AddTestToQueueWithConfirmation/queries.js | 23 ++- .../DataManagementRow/index.jsx | 38 +++-- client/components/TestQueue/queries.js | 2 + .../TestPlanReportStatusDialogMock.js | 12 +- .../GraphQLMocks/TestQueuePageBaseMock.js | 15 +- server/graphql-schema.js | 6 +- .../updatePhaseResolver.js | 9 +- .../findOrCreateTestPlanReportResolver.js | 44 +++++ .../resolvers/helpers/processCopiedReports.js | 22 ++- server/resolvers/testPlanVersionsResolver.js | 8 +- .../tests/integration/dataManagement.test.js | 14 +- 12 files changed, 268 insertions(+), 79 deletions(-) diff --git a/client/components/AddTestToQueueWithConfirmation/index.jsx b/client/components/AddTestToQueueWithConfirmation/index.jsx index 637a7d2ee..4add46dfc 100644 --- a/client/components/AddTestToQueueWithConfirmation/index.jsx +++ b/client/components/AddTestToQueueWithConfirmation/index.jsx @@ -23,15 +23,18 @@ function AddTestToQueueWithConfirmation({ buttonText = 'Add to Test Queue', triggerUpdate = () => {} }) { - const [errorMessage, setErrorMessage] = useState(false); + const [showPreserveReportDataMessage, setShowPreserveReportDataMessage] = + useState(false); const [showConfirmation, setShowConfirmation] = useState(false); + const [canUseOldResults, setCanUseOldResults] = useState(false); const [addTestPlanReport] = useMutation(ADD_TEST_QUEUE_MUTATION); const [scheduleCollection] = useMutation(SCHEDULE_COLLECTION_JOB_MUTATION); const { data: existingTestPlanReportsData } = useQuery( EXISTING_TEST_PLAN_REPORTS, { variables: { - testPlanVersionId: testPlanVersion?.id + testPlanVersionId: testPlanVersion?.id, + directory: testPlanVersion?.testPlan?.directory }, fetchPolicy: 'cache-and-network', skip: !testPlanVersion?.id @@ -39,7 +42,7 @@ function AddTestToQueueWithConfirmation({ ); const existingTestPlanReports = - existingTestPlanReportsData?.testPlanVersion.testPlanReports; + existingTestPlanReportsData?.existingTestPlanVersion?.testPlanReports; const conflictingReportExists = existingTestPlanReports?.some(report => { return ( @@ -49,6 +52,31 @@ function AddTestToQueueWithConfirmation({ ); }); + let latestOldVersion; + let oldReportToCopyResultsFrom; + + // Prioritize a conflicting report for the current version, otherwise + // check if any results data available from a previous result + if ( + !conflictingReportExists && + existingTestPlanReportsData?.oldTestPlanVersions?.length + ) { + latestOldVersion = + existingTestPlanReportsData?.oldTestPlanVersions?.reduce((a, b) => + new Date(a.updatedAt) > new Date(b.updatedAt) ? a : b + ); + + if ( + new Date(latestOldVersion?.updatedAt) < + new Date(testPlanVersion?.updatedAt) + ) { + oldReportToCopyResultsFrom = latestOldVersion?.testPlanReports.some( + ({ at: { id: atId }, browser: { id: browserId } }) => + atId == at?.id && browserId == browser?.id + ); + } + } + const { triggerLoad, loadingMessage } = useTriggerLoad(); const buttonRef = useRef(); @@ -104,7 +132,14 @@ function AddTestToQueueWithConfirmation({ actions.push({ label: 'Add and run later', onClick: async () => { - await addTestToQueue(); + await addTestToQueue( + canUseOldResults + ? { + copyResultsFromTestPlanReportId: + latestOldVersion.id + } + : {} + ); await closeWithUpdate(); } }); @@ -113,7 +148,14 @@ function AddTestToQueueWithConfirmation({ actions.push({ label: 'Add and run with bot', onClick: async () => { - const testPlanReport = await addTestToQueue(); + const testPlanReport = await addTestToQueue( + canUseOldResults + ? { + copyResultsFromTestPlanReportId: + latestOldVersion.id + } + : {} + ); await scheduleCollectionJob(testPlanReport); await closeWithUpdate(); } @@ -141,48 +183,94 @@ function AddTestToQueueWithConfirmation({ ); }; - const renderErrorDialog = () => { + const renderPreserveReportDataDialog = () => { + let title; + let content; + let actions = []; + + if (oldReportToCopyResultsFrom) { + title = 'Older Results Data Found'; + content = + 'Older results with the same AT, browser and test plan ' + + 'were found for the report being created. Would you like ' + + 'to copy the older results into the report or create a ' + + 'completely new report?'; + actions = [ + { + label: 'Create empty report', + onClick: async () => { + setShowPreserveReportDataMessage(false); + if (hasAutomationSupport) { + setShowConfirmation(true); + } else { + await addTestToQueue(); + } + } + }, + { + label: 'Copy older results', + onClick: async () => { + setShowPreserveReportDataMessage(false); + setCanUseOldResults(true); + + if (hasAutomationSupport) { + setShowConfirmation(true); + } else { + await addTestToQueue({ + copyResultsFromTestPlanReportId: + latestOldVersion.id + }); + } + } + } + ]; + } else { + title = 'Conflicting Report Found'; + content = + 'The report could not be created because an existing ' + + 'report was found on the reports page with the same AT, ' + + 'browser and test plan version. Would you like to return ' + + 'the existing report back to the test queue?'; + actions = [ + { + label: 'Proceed', + onClick: async () => { + setShowPreserveReportDataMessage(false); + if (hasAutomationSupport) { + setShowConfirmation(true); + } else { + await addTestToQueue(); + } + } + } + ]; + } + return ( { - setErrorMessage(false); - if (hasAutomationSupport) { - setShowConfirmation(true); - } else { - await addTestToQueue(); - } - } - } - ]} + actions={actions} useOnHide handleClose={async () => { - setErrorMessage(false); + setShowPreserveReportDataMessage(false); }} /> ); }; - const addTestToQueue = async () => { + const addTestToQueue = async ({ copyResultsFromTestPlanReportId } = {}) => { let tpr; await triggerLoad(async () => { const res = await addTestPlanReport({ variables: { testPlanVersionId: testPlanVersion.id, atId: at.id, - browserId: browser.id + browserId: browser.id, + copyResultsFromTestPlanReportId } }); const testPlanReport = @@ -212,8 +300,8 @@ function AddTestToQueueWithConfirmation({ disabled={disabled} variant="secondary" onClick={async () => { - if (conflictingReportExists) { - setErrorMessage(true); + if (conflictingReportExists || oldReportToCopyResultsFrom) { + setShowPreserveReportDataMessage(true); } else { if (hasAutomationSupport) { setShowConfirmation(true); @@ -227,7 +315,7 @@ function AddTestToQueueWithConfirmation({ > {buttonText} - {renderErrorDialog()} + {renderPreserveReportDataDialog()} {renderConfirmation()} ); diff --git a/client/components/AddTestToQueueWithConfirmation/queries.js b/client/components/AddTestToQueueWithConfirmation/queries.js index 796098849..e7d7b40d3 100644 --- a/client/components/AddTestToQueueWithConfirmation/queries.js +++ b/client/components/AddTestToQueueWithConfirmation/queries.js @@ -10,8 +10,11 @@ export const SCHEDULE_COLLECTION_JOB_MUTATION = gql` `; export const EXISTING_TEST_PLAN_REPORTS = gql` - query ExistingTestPlanReports($testPlanVersionId: ID!) { - testPlanVersion(id: $testPlanVersionId) { + query ExistingTestPlanReports( + $testPlanVersionId: ID! + $directory: String! + ) { + existingTestPlanVersion: testPlanVersion(id: $testPlanVersionId) { id testPlanReports { id @@ -28,5 +31,21 @@ export const EXISTING_TEST_PLAN_REPORTS = gql` } } } + oldTestPlanVersions: testPlanVersions( + phases: [CANDIDATE, RECOMMENDED] + directory: $directory + ) { + id + updatedAt + testPlanReports { + id + at { + id + } + browser { + id + } + } + } } `; diff --git a/client/components/DataManagement/DataManagementRow/index.jsx b/client/components/DataManagement/DataManagementRow/index.jsx index 716afd8ed..c3b8d777c 100644 --- a/client/components/DataManagement/DataManagementRow/index.jsx +++ b/client/components/DataManagement/DataManagementRow/index.jsx @@ -572,18 +572,18 @@ const DataManagementRow = ({ } } - // If there is an earlier version that is draft and that version has some test plan + // If there is an earlier version that is draft or higher, and that version has some test plan // runs in the test queue, this button will run the process for updating existing // reports and preserving data for tests that have not changed. let testPlanVersionDataToInclude; - if (draftTestPlanVersions.length) { + if (otherTestPlanVersions.length) { const { - latestVersion: draftLatestVersion, - latestVersionDate: draftLatestVersionDate - } = getVersionData(draftTestPlanVersions); + latestVersion: otherLatestVersion, + latestVersionDate: otherLatestVersionDate + } = getVersionData(otherTestPlanVersions); - if (draftLatestVersionDate < latestVersionDate) - testPlanVersionDataToInclude = draftLatestVersion; + if (otherLatestVersionDate < latestVersionDate) + testPlanVersionDataToInclude = otherLatestVersion; } // Otherwise, show VERSION_STRING link with a draft transition button. Phase is @@ -688,20 +688,28 @@ const DataManagementRow = ({ // If required reports are complete and user is an admin, show "Advance to // Candidate" button. if (testPlanVersions.length) { - // If there is an earlier version that is candidate and that version has some + // If there is an earlier version that is candidate or higher and that version has some // test plan runs in the test queue, this button will run the process for // updating existing reports and preserving data for tests that have not // changed. let testPlanVersionDataToInclude; - if (candidateTestPlanVersions.length) { + const testPlanVersionsDataToInclude = [ + ...candidateTestPlanVersions, + ...recommendedTestPlanVersions + ]; + if (testPlanVersionsDataToInclude.length) { const { - latestVersion: candidateLatestVersion, - latestVersionDate: candidateLatestVersionDate - } = getVersionData(candidateTestPlanVersions); - - if (candidateLatestVersionDate < latestVersionDate) + latestVersion: testPlanDataToIncludeLatestVersion, + latestVersionDate: + testPlanDataToIncludeLatestVersionDate + } = getVersionData(testPlanVersionsDataToInclude); + + if ( + testPlanDataToIncludeLatestVersionDate < + latestVersionDate + ) testPlanVersionDataToInclude = - candidateLatestVersion; + testPlanDataToIncludeLatestVersion; } let coveredReports = []; diff --git a/client/components/TestQueue/queries.js b/client/components/TestQueue/queries.js index faaf8300d..0f5ca2375 100644 --- a/client/components/TestQueue/queries.js +++ b/client/components/TestQueue/queries.js @@ -222,12 +222,14 @@ export const ADD_TEST_QUEUE_MUTATION = gql` $testPlanVersionId: ID! $atId: ID! $browserId: ID! + $copyResultsFromTestPlanReportId: ID ) { findOrCreateTestPlanReport( input: { testPlanVersionId: $testPlanVersionId atId: $atId browserId: $browserId + copyResultsFromTestPlanReportId: $copyResultsFromTestPlanReportId } ) { populatedData { diff --git a/client/tests/__mocks__/GraphQLMocks/TestPlanReportStatusDialogMock.js b/client/tests/__mocks__/GraphQLMocks/TestPlanReportStatusDialogMock.js index 2ee226bf3..3dee6df1b 100644 --- a/client/tests/__mocks__/GraphQLMocks/TestPlanReportStatusDialogMock.js +++ b/client/tests/__mocks__/GraphQLMocks/TestPlanReportStatusDialogMock.js @@ -282,7 +282,7 @@ export const mockedTestPlanVersion = { export default ( meQuery, testPlanReportStatusDialogQuery, - initiatedByAutomationQuery + existingTestPlanReportsQuery ) => [ { request: { @@ -386,14 +386,15 @@ export default ( }, { request: { - query: initiatedByAutomationQuery, + query: existingTestPlanReportsQuery, variables: { - testPlanVersionId: '7' + testPlanVersionId: '7', + directory: 'combobox-select-only' } }, result: { data: { - testPlanVersion: { + existingTestPlanVersion: { id: '7', testPlanReports: [ { @@ -411,7 +412,8 @@ export default ( } } ] - } + }, + oldTestPlanVersions: [] } } } diff --git a/client/tests/__mocks__/GraphQLMocks/TestQueuePageBaseMock.js b/client/tests/__mocks__/GraphQLMocks/TestQueuePageBaseMock.js index adef2575c..2aed220dc 100644 --- a/client/tests/__mocks__/GraphQLMocks/TestQueuePageBaseMock.js +++ b/client/tests/__mocks__/GraphQLMocks/TestQueuePageBaseMock.js @@ -1,7 +1,4 @@ -export default ( - testPlanReportAtBrowserQuery, - initiatedByAutomationRunQuery -) => [ +export default (testPlanReportAtBrowserQuery, existingTestPlanReportsQuery) => [ { request: { query: testPlanReportAtBrowserQuery, @@ -73,14 +70,15 @@ export default ( }, { request: { - query: initiatedByAutomationRunQuery, + query: existingTestPlanReportsQuery, variables: { - testPlanVersionId: '1' + testPlanVersionId: '1', + directory: 'alert' } }, result: { data: { - testPlanVersion: { + existingTestPlanVersion: { id: '1', testPlanReports: [ { @@ -98,7 +96,8 @@ export default ( } } ] - } + }, + oldTestPlanVersions: [] } } } diff --git a/server/graphql-schema.js b/server/graphql-schema.js index fb818822d..92cf9d25a 100644 --- a/server/graphql-schema.js +++ b/server/graphql-schema.js @@ -1016,6 +1016,7 @@ const graphqlSchema = gql` testPlanVersionId: ID! atId: ID! browserId: ID! + copyResultsFromTestPlanReportId: ID } """ @@ -1101,7 +1102,10 @@ const graphqlSchema = gql` """ Get all TestPlanVersions. """ - testPlanVersions(phases: [TestPlanVersionPhase]): [TestPlanVersion]! + testPlanVersions( + phases: [TestPlanVersionPhase] + directory: String + ): [TestPlanVersion]! """ Get a particular TestPlanVersion by ID. """ diff --git a/server/resolvers/TestPlanVersionOperations/updatePhaseResolver.js b/server/resolvers/TestPlanVersionOperations/updatePhaseResolver.js index e57fa7a60..0e9dd551a 100644 --- a/server/resolvers/TestPlanVersionOperations/updatePhaseResolver.js +++ b/server/resolvers/TestPlanVersionOperations/updatePhaseResolver.js @@ -316,14 +316,15 @@ const updatePhaseResolver = async ( deprecatedAt: null }; - // If testPlanVersionDataToIncludeId's results are being used to update this earlier version, - // deprecate it - if (testPlanVersionDataToIncludeId) + // If oldTestPlanVersion's results are being used to update this earlier + // version, deprecate it (if the same phase) + if (oldTestPlanVersion && phase === oldTestPlanVersion.phase) { await updateTestPlanVersionById({ - id: testPlanVersionDataToIncludeId, + id: oldTestPlanVersion.id, // same as testPlanVersionDataToIncludeId values: { phase: 'DEPRECATED', deprecatedAt: new Date() }, transaction }); + } await updateTestPlanVersionById({ id: testPlanVersionId, diff --git a/server/resolvers/findOrCreateTestPlanReportResolver.js b/server/resolvers/findOrCreateTestPlanReportResolver.js index 303c2d41c..a056fe21a 100644 --- a/server/resolvers/findOrCreateTestPlanReportResolver.js +++ b/server/resolvers/findOrCreateTestPlanReportResolver.js @@ -3,6 +3,7 @@ const { getOrCreateTestPlanReport } = require('../models/services/TestPlanReportService'); const populateData = require('../services/PopulatedData/populateData'); +const processCopiedReports = require('./helpers/processCopiedReports'); const findOrCreateTestPlanReportResolver = async (_, { input }, context) => { const { user, transaction } = context; @@ -11,6 +12,49 @@ const findOrCreateTestPlanReportResolver = async (_, { input }, context) => { throw new AuthenticationError(); } + // Pull back report from TestPlanVersion in advanced phase and run through processCopiedReports if not deprecated + const { copyResultsFromTestPlanReportId } = input; + + if (copyResultsFromTestPlanReportId) { + const { updatedTestPlanReports } = await processCopiedReports({ + oldTestPlanVersionId: copyResultsFromTestPlanReportId, + newTestPlanVersionId: input.testPlanVersionId, + newTestPlanReports: [], + atBrowserCombinationsToInclude: [ + { atId: input.atId, browserId: input.browserId } + ], + context + }); + + if (updatedTestPlanReports?.length) { + // Expecting only to get back the single requested combination + const [testPlanReport] = updatedTestPlanReports; + + const locationOfData = { + testPlanReportId: testPlanReport.id + }; + const preloaded = { + testPlanReport + }; + const createdLocationsOfData = [locationOfData]; + + return { + populatedData: await populateData(locationOfData, { + preloaded, + context + }), + created: await Promise.all( + createdLocationsOfData.map(createdLocationOfData => + populateData(createdLocationOfData, { + preloaded, + context + }) + ) + ) + }; + } + } + const [testPlanReport, createdLocationsOfData] = await getOrCreateTestPlanReport({ where: input, transaction }); diff --git a/server/resolvers/helpers/processCopiedReports.js b/server/resolvers/helpers/processCopiedReports.js index da3244d7c..52e059648 100644 --- a/server/resolvers/helpers/processCopiedReports.js +++ b/server/resolvers/helpers/processCopiedReports.js @@ -243,6 +243,7 @@ const processCopiedReports = async ({ oldTestPlanVersionId, newTestPlanVersionId, newTestPlanReports, + atBrowserCombinationsToInclude = [], context }) => { const { transaction } = context; @@ -289,6 +290,10 @@ const processCopiedReports = async ({ }; } + const atBrowserStringCombinations = atBrowserCombinationsToInclude.map( + ({ atId, browserId }) => `${atId}_${browserId}` + ); + // If there is an earlier version that for this phase and that version has some test plan runs // in the test queue, this will run the process for updating existing test plan versions for the // test plan version and preserving data for tests that have not changed. @@ -305,6 +310,17 @@ const processCopiedReports = async ({ continue; } + if ( + atBrowserStringCombinations.length && + !atBrowserStringCombinations.find( + combination => + combination === + `${oldTestPlanReport.atId}_${oldTestPlanReport.browserId}` + ) + ) { + continue; + } + // Then the combination needs to be considered if the tests are not different // between versions const keptTestsByTestHash = getKeptTestsByTestHash( @@ -450,7 +466,11 @@ const processCopiedReports = async ({ return { oldTestPlanVersion, newTestPlanReportIds, - updatedTestPlanReports + updatedTestPlanReports: atBrowserStringCombinations.length + ? updatedTestPlanReports.filter(({ atId, browserId }) => + atBrowserStringCombinations.includes(`${atId}_${browserId}`) + ) + : updatedTestPlanReports }; }; diff --git a/server/resolvers/testPlanVersionsResolver.js b/server/resolvers/testPlanVersionsResolver.js index 4633ab0fd..543b4ca37 100644 --- a/server/resolvers/testPlanVersionsResolver.js +++ b/server/resolvers/testPlanVersionsResolver.js @@ -4,11 +4,17 @@ const { const retrieveAttributes = require('./helpers/retrieveAttributes'); const { TEST_PLAN_VERSION_ATTRIBUTES } = require('../models/services/helpers'); -const testPlanVersionsResolver = async (_, { phases }, context, info) => { +const testPlanVersionsResolver = async ( + _, + { phases, directory }, + context, + info +) => { const { transaction } = context; const where = {}; if (phases) where.phase = phases; + if (directory) where.directory = directory; const { attributes: testPlanVersionAttributes } = retrieveAttributes( 'testPlanVersion', diff --git a/server/tests/integration/dataManagement.test.js b/server/tests/integration/dataManagement.test.js index 0e5cb5740..83ae6c893 100644 --- a/server/tests/integration/dataManagement.test.js +++ b/server/tests/integration/dataManagement.test.js @@ -332,14 +332,14 @@ describe('data management', () => { }); }); - it('updates test plan version and copies results from previous version reports', async () => { + it('updates test plan version and copies results from previous version reports even in advanced phase', async () => { await dbCleaner(async transaction => { const testPlanVersions = await testPlanVersionsQuery({ transaction }); // This has reports for JAWS + Chrome, NVDA + Chrome, VO + Safari + additional - // non-required reports + // non-required reports in CANDIDATE const [oldModalDialogVersion] = testPlanVersions.testPlanVersions.filter( e => @@ -347,13 +347,6 @@ describe('data management', () => { e.phase === 'CANDIDATE' ); - // This version is in 'CANDIDATE' phase. Set it to DRAFT - // This will also remove the associated TestPlanReports markedFinalAt values - const oldTestPlanVersionId = oldModalDialogVersion.id; - await updateVersionToPhaseQuery(oldTestPlanVersionId, 'DRAFT', { - transaction - }); - const oldModalDialogVersionTestPlanReports = oldModalDialogVersion.testPlanReports; const oldModalDialogVersionTestPlanReportsCount = @@ -383,6 +376,7 @@ describe('data management', () => { ) ); + // Process new version coming in as RD const [newModalDialogVersion] = testPlanVersions.testPlanVersions.filter( e => @@ -392,6 +386,8 @@ describe('data management', () => { const newModalDialogVersionTestPlanReportsInRDCount = newModalDialogVersion.testPlanReports.length; + // Should still retrieve results from older CANDIDATE version since + // no DRAFT version was present const { testPlanVersion: newModalDialogVersionInDraft } = await updateVersionToPhaseQuery( newModalDialogVersion.id,