Skip to content

Commit

Permalink
Enhanced conflict review [extended] (#1197)
Browse files Browse the repository at this point in the history 
* 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 <[email protected]>
  • Loading branch information
@howard-e @stalgiag
howard-e and stalgiag authored Aug 26, 2024
1 parent 97d893a commit 4eded57
Show file tree
Hide file tree
Showing 7 changed files with 212 additions and 50 deletions.
2 changes: 1 addition & 1 deletion client/components/TestQueue/AssignTesters.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
191 changes: 172 additions & 19 deletions client/components/TestQueue/Conflicts/index.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)`
Expand Down Expand Up @@ -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',
Expand All @@ -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]);

Expand All @@ -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 ===
Expand All @@ -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 (
Expand All @@ -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 (
<PageContainer id="main" as="main" tabIndex="-1">
<Helmet>
Expand Down Expand Up @@ -199,8 +348,8 @@ const TestQueueConflicts = () => {
<MetadataItem>
<MetadataLabel>Assistive Technology:</MetadataLabel>
{atName}
{requiredAtVersionName
? ` (${requiredAtVersionName})`
{exactAtVersionName
? ` (${exactAtVersionName})`
: ` (${minimumAtVersionName} and above)`}
</MetadataItem>
<MetadataItem>
Expand All @@ -215,6 +364,10 @@ const TestQueueConflicts = () => {
<ConflictCount>
There are currently
<strong> {data?.testPlanReport?.conflicts?.length} conflicts </strong>
across
<strong> {uniqueTestsLength} tests </strong>
and
<strong> {uniqueConflictsByAssertion.length} assertions </strong>
for this test plan report.
</ConflictCount>
<DisclosureComponent
Expand Down
5 changes: 5 additions & 0 deletions client/components/TestQueue/queries.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,11 @@ export const TEST_QUEUE_CONFLICTS_PAGE_QUERY = gql`
title
renderedUrl
}
scenario {
commands {
text
}
}
scenarioResult {
output
unexpectedBehaviors {
Expand Down
4 changes: 2 additions & 2 deletions client/tests/e2e/snapshots/saved/_data-management.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1087,7 +1087,7 @@ <h2>Test Plans Status Summary</h2>
<td>
<div class="css-bpz90">
<span class="rd full-width css-be9e2a">R&amp;D</span>
<p class="review-text">Complete <b>Aug 12, 2024</b></p>
<p class="review-text">Complete <b>Aug 21, 2024</b></p>
</div>
</td>
<td>
Expand All @@ -1110,7 +1110,7 @@ <h2>Test Plans Status Summary</h2>
<path
fill="currentColor"
d="M256 512A256 256 0 1 0 256 0a256 256 0 1 0 0 512zM369 209L241 337c-9.4 9.4-24.6 9.4-33.9 0l-64-64c-9.4-9.4-9.4-24.6 0-33.9s24.6-9.4 33.9 0l47 47L335 175c9.4-9.4 24.6-9.4 33.9 0s9.4 24.6 0 33.9z"></path></svg
><b>V24.08.12</b></span
><b>V24.08.21</b></span
></a
></span
><button
Expand Down
Loading

0 comments on commit 4eded57

Please sign in to comment.