From 4eded578e7bd3ac21019147cd46e21a3d5825944 Mon Sep 17 00:00:00 2001 From: Howard Edwards Date: Mon, 26 Aug 2024 11:22:12 -0400 Subject: [PATCH] Enhanced conflict review [extended] (#1197) * Address sandbox deployment issue (#1190) * Changes to pass sandbox deployment issues * Remove explicit install of npm during deploy * fix: Only match with single alphanumeric keys in "Assign Testers" dropdown search, ignore Tab (#1196) * * undefined requiredAtVersionName renamed to exactAtVersionName * Re-use versionString instead of relying on testPlanVersion.updatedAt (noticed because it wasn't being passed to the created GH Issue) * Create distinction between each conflict * Create unique disclosures * Correct the conflicts to be unique across assertions * Support including conflict markdown from conflicts issue creation * Update snapshot --------- Co-authored-by: Stalgia Grigg --- client/components/TestQueue/AssignTesters.jsx | 2 +- .../components/TestQueue/Conflicts/index.jsx | 191 ++++++++++++++++-- client/components/TestQueue/queries.js | 5 + .../e2e/snapshots/saved/_data-management.html | 4 +- .../saved/_test-queue_2_conflicts.html | 55 ++--- deploy/roles/application/tasks/main.yml | 4 + deploy/roles/nodejs/tasks/main.yml | 1 - 7 files changed, 212 insertions(+), 50 deletions(-) diff --git a/client/components/TestQueue/AssignTesters.jsx b/client/components/TestQueue/AssignTesters.jsx index 7e2c64c1a..bd8e40c6f 100644 --- a/client/components/TestQueue/AssignTesters.jsx +++ b/client/components/TestQueue/AssignTesters.jsx @@ -94,7 +94,7 @@ const AssignTesters = ({ me, testers, testPlanReport }) => { const onKeyDown = event => { const { key } = event; - if (key.match(/[0-9a-zA-Z]/)) { + if (key.length === 1 && key.match(/[a-zA-Z0-9]/)) { const container = event.target.closest('[role=menu]'); const matchingMenuItem = Array.from(container.children).find(menuItem => { return menuItem.innerText diff --git a/client/components/TestQueue/Conflicts/index.jsx b/client/components/TestQueue/Conflicts/index.jsx index d5f34eaf9..ba2bf2751 100644 --- a/client/components/TestQueue/Conflicts/index.jsx +++ b/client/components/TestQueue/Conflicts/index.jsx @@ -3,13 +3,12 @@ import React, { useEffect, useMemo, useState } from 'react'; import { Container } from 'react-bootstrap'; import { Helmet } from 'react-helmet'; import { useParams } from 'react-router-dom'; +import styled from '@emotion/styled'; import { TEST_QUEUE_CONFLICTS_PAGE_QUERY } from '../queries'; import PageStatus from '../../common/PageStatus'; import DisclosureComponent from '../../common/DisclosureComponent'; import ConflictSummaryTable from './ConflictSummaryTable'; import createIssueLink from '../../../utils/createIssueLink'; -import { dates } from 'shared'; -import styled from '@emotion/styled'; import { evaluateAuth } from '../../../utils/evaluateAuth'; const PageContainer = styled(Container)` @@ -57,6 +56,10 @@ const ConflictCount = styled.p` const TestQueueConflicts = () => { const [openDisclosures, setOpenDisclosures] = useState([]); + const [uniqueConflictsByAssertion, setUniqueConflictsByAssertion] = useState( + [] + ); + const { testPlanReportId } = useParams(); const { data, error, loading } = useQuery(TEST_QUEUE_CONFLICTS_PAGE_QUERY, { fetchPolicy: 'cache-and-network', @@ -67,7 +70,35 @@ const TestQueueConflicts = () => { useEffect(() => { if (data) { - setOpenDisclosures(data.testPlanReport.conflicts.map(() => false)); + // Each conflict per test is counted so it could cause duplicate + // disclosures + // + // eg. tester1 and tester2 marking 2 assertions in test1 the opposite way + // would create 2 disclosures + const createUniqueConflictId = (testId = '', commands = []) => + `${testId}-${commands.map(({ text }) => text).join('')}`; + + const uniqueConflictsByAssertions = []; + data?.testPlanReport?.conflicts?.map(conflict => { + const conflictId = createUniqueConflictId( + conflict.conflictingResults[0].test.id, + conflict.conflictingResults[0].scenario.commands + ); + + if ( + !uniqueConflictsByAssertions.find( + conflict => + createUniqueConflictId( + conflict.conflictingResults[0].test.id, + conflict.conflictingResults[0].scenario.commands + ) === conflictId + ) + ) { + uniqueConflictsByAssertions.push(conflict); + } + }); + setUniqueConflictsByAssertion(uniqueConflictsByAssertions); + setOpenDisclosures(uniqueConflictsByAssertions.map(() => false)); } }, [data]); @@ -81,35 +112,147 @@ const TestQueueConflicts = () => { const getIssueLink = conflict => { if (!conflict) return; + let markdownConflictCount = 1; + // Standardize on format currently supported in ReviewConflicts + let conflictMarkdown = ` +## Review Conflicts for "${conflict.conflictingResults[0].test.title}"`; + + // Setup conflicting assertions handling + const conflictingAssertions = []; + conflict.conflictingResults[0].scenarioResult.assertionResults.forEach( + (ar, index) => { + conflict.conflictingResults.forEach(cr => { + const hasConflict = + cr.scenarioResult.assertionResults[index].passed !== ar.passed; + + if (hasConflict) { + conflictingAssertions.push({ + index, + assertion: cr.scenarioResult.assertionResults[index].assertion + }); + } + }); + } + ); + const hasAssertionConflicts = conflictingAssertions.length > 0; + + if (hasAssertionConflicts) { + for (const conflictingAssertion of conflictingAssertions) { + // Standardize on format currently supported in ReviewConflicts + conflictMarkdown = `${conflictMarkdown} + +${markdownConflictCount}. ### Assertion Results for "${conflict.conflictingResults[0].scenario.commands + .map(({ text }) => text) + .join(' then ')}" Command and "${ + conflictingAssertion.assertion.text + }" Assertion`; + markdownConflictCount++; + + conflict.conflictingResults.forEach(result => { + const { testPlanRun, scenarioResult } = result; + let assertionResultFormatted; + assertionResultFormatted = scenarioResult.assertionResults[ + conflictingAssertion.index + ].passed + ? 'passing' + : 'failing'; + + conflictMarkdown = `${conflictMarkdown} +- Tester ${testPlanRun.tester.username} recorded output "${scenarioResult.output}" and marked assertion as ${assertionResultFormatted}`; + }); + } + } + + // Setup conflicting unexpected behaviors handling + const conflictingUnexpectedBehaviorsByTester = {}; + conflict.conflictingResults.forEach(result => { + result.scenarioResult.unexpectedBehaviors.forEach( + ({ text, details, impact }) => { + const username = result.testPlanRun.tester.username; + const conflict = { + text, + details, + impact, + output: result.scenarioResult.output, + tester: result.testPlanRun.tester, + command: result.scenario.commands + .map(({ text }) => text) + .join(' then ') + }; + + if (!conflictingUnexpectedBehaviorsByTester[username]) { + conflictingUnexpectedBehaviorsByTester[username] = [conflict]; + } else { + conflictingUnexpectedBehaviorsByTester[username].push(conflict); + } + } + ); + }); + const hasUnexpectedBehaviorConflicts = + Object.keys(conflictingUnexpectedBehaviorsByTester).length > 0; + + if (hasUnexpectedBehaviorConflicts) { + conflictMarkdown = `${conflictMarkdown} + +${markdownConflictCount}. ### Unexpected Behaviors Results for "${conflict.conflictingResults[0].scenario.commands + .map(({ text }) => text) + .join(' then ')}" Command`; + markdownConflictCount++; + + for (const key in conflictingUnexpectedBehaviorsByTester) { + const unexpectedBehaviorsForTester = + conflictingUnexpectedBehaviorsByTester[key]; + + const commonUnexpectedBehaviorForTester = + unexpectedBehaviorsForTester[0]; + const unexpectedBehaviorText = unexpectedBehaviorsForTester + .map((unexpectedBehavior, index) => { + let note = `"${unexpectedBehavior.text} (Details: ${unexpectedBehavior.details}, Impact: ${unexpectedBehavior.impact})"`; + if (index + 1 < unexpectedBehaviorsForTester.length) + note = `${note} and`; + + return note; + }) + .join(' '); + + conflictMarkdown = `${conflictMarkdown} +- Tester ${commonUnexpectedBehaviorForTester.tester.username} recorded output "${commonUnexpectedBehaviorForTester.output}" and noted ${unexpectedBehaviorText}`; + } + } + const { testPlanVersion } = data.testPlanReport; return createIssueLink({ testPlanTitle: testPlanVersion.title, testPlanDirectory: testPlanVersion.testPlan.directory, - versionString: `V${dates.convertDateToString( - testPlanVersion.updatedAt, - 'YY.MM.DD' - )}`, + versionString: testPlanVersion.versionString, testTitle: conflict.conflictingResults[0].test.title, testRowNumber: conflict.conflictingResults[0].test.rowNumber, testSequenceNumber: getConflictTestNumberFilteredByAt(conflict), testRenderedUrl: conflict.conflictingResults[0].test.renderedUrl, atName: data.testPlanReport.at.name, browserName: data.testPlanReport.browser.name, - atVersionName: data.testPlanReport.requiredAtVersion?.name + atVersionName: data.testPlanReport.exactAtVersion?.name + ? data.testPlanReport.exactAtVersion?.name + : `${data.testPlanReport.minimumAtVersion?.name} and above`, + conflictMarkdown }); }; const { isAdmin } = useMemo(() => evaluateAuth(data?.me), [data?.me]); const disclosureLabels = useMemo(() => { - return data?.testPlanReport?.conflicts?.map(conflict => { + return uniqueConflictsByAssertion.map(conflict => { const testIndex = getConflictTestNumberFilteredByAt(conflict); - return `Test ${testIndex}: ${conflict.conflictingResults[0].test.title}`; + return `Test ${testIndex}: ${ + conflict.conflictingResults[0].test.title + } (${conflict.conflictingResults[0].scenario.commands + .map(({ text }) => text) + .join(' then ')})`; }); - }, [data?.testPlanReport?.conflicts]); + }, [uniqueConflictsByAssertion]); const disclosureContents = useMemo(() => { - return data?.testPlanReport?.conflicts?.map(conflict => { + return uniqueConflictsByAssertion.map(conflict => { const issues = data?.testPlanReport?.issues?.filter( issue => issue.testNumberFilteredByAt === @@ -126,17 +269,17 @@ const TestQueueConflicts = () => { /> ); }); - }, [data?.testPlanReport?.conflicts]); + }, [uniqueConflictsByAssertion]); const disclosureClickHandlers = useMemo(() => { - return data?.testPlanReport?.conflicts?.map((_, index) => () => { + return uniqueConflictsByAssertion.map((_, index) => () => { setOpenDisclosures(prevState => { const newOpenDisclosures = [...prevState]; newOpenDisclosures[index] = !newOpenDisclosures[index]; return newOpenDisclosures; }); }); - }, [data?.testPlanReport?.conflicts]); + }, [uniqueConflictsByAssertion]); if (error) { return ( @@ -162,11 +305,17 @@ const TestQueueConflicts = () => { } = data?.testPlanReport.testPlanVersion ?? {}; const { name: browserName } = data?.testPlanReport.browser ?? {}; const { name: atName } = data?.testPlanReport.at ?? {}; - const { name: requiredAtVersionName } = - data?.testPlanReport.requiredAtVersion ?? {}; + const { name: exactAtVersionName } = + data?.testPlanReport.exactAtVersion ?? {}; const { name: minimumAtVersionName } = data?.testPlanReport.minimumAtVersion ?? {}; + const uniqueTestsLength = new Set( + data?.testPlanReport?.conflicts.map( + conflict => conflict.conflictingResults[0].test.id + ) + ).size; + return ( @@ -199,8 +348,8 @@ const TestQueueConflicts = () => { Assistive Technology: {atName} - {requiredAtVersionName - ? ` (${requiredAtVersionName})` + {exactAtVersionName + ? ` (${exactAtVersionName})` : ` (${minimumAtVersionName} and above)`} @@ -215,6 +364,10 @@ const TestQueueConflicts = () => { There are currently {data?.testPlanReport?.conflicts?.length} conflicts + across + {uniqueTestsLength} tests + and + {uniqueConflictsByAssertion.length} assertions for this test plan report. Test Plans Status Summary
R&D -

Complete Aug 12, 2024

+

Complete Aug 21, 2024

@@ -1110,7 +1110,7 @@

Test Plans Status Summary

V24.08.12V24.08.21