Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update undesirable behavior impact collection details #939

Merged
merged 9 commits into from
Mar 6, 2024
Merged

Update undesirable behavior impact collection details #939

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
f1611f5
Fix issue undesirable behavior checkboxes not being able to be tabbed…
howard-e Feb 27, 2024
51fda5e
Change references of 'HIGH' impact to 'SEVERE' impact
howard-e Feb 27, 2024
c120503
Update how undesirable behaviors are displayed in Test Results Table
howard-e Feb 27, 2024
38728d6
Merge branch 'main' into update-impact-collection-details
howard-e Feb 28, 2024
cde9ced
Update getMetrics after merging shared workspace
howard-e Feb 28, 2024
ddba40f
Address odd edge case where behavior column is blank on Test Run page…
howard-e Feb 29, 2024
f68ab93
Address PR feedback
howard-e Feb 29, 2024
cd25da1
Merge branch 'main' into update-impact-collection-details
howard-e Feb 29, 2024
d29bbc9
Merge branch 'main' into update-impact-collection-details
howard-e Mar 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions client/components/TestRenderer/UnexpectedBehaviorsFieldset/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ const UnexpectedBehaviorsFieldset = ({
unexpectedBehaviors,
isSubmitted
}) => {
const impactOptions = ['Moderate', 'High'];
const impactOptions = ['Moderate', 'Severe'];

return (
<Fieldset id={`cmd-${commandIndex}-problems`}>
Expand Down Expand Up @@ -135,7 +135,6 @@ const UnexpectedBehaviorsFieldset = ({
type="checkbox"
value={description}
className={`undesirable-${commandIndex}`}
tabIndex={optionIndex === 0 ? 0 : -1}
autoFocus={isSubmitted && focus}
checked={checked}
onChange={e => change(e.target.checked)}
Expand Down
4 changes: 4 additions & 0 deletions client/components/common/TestPlanResultsTable/TestPlanResultsTable.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ table.test-plan-results-table {
caption-side: unset;
}

table.test-plan-undesirable-behaviors-table {
margin-top: 0.5rem;
}

p.test-plan-results-response-p {
margin: 0;
}
Expand Down
54 changes: 32 additions & 22 deletions client/components/common/TestPlanResultsTable/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ const TestPlanResultsTable = ({
const failedAssertions = scenarioResult.assertionResults.filter(
assertionResult => !assertionResult.passed
);
const hasNoHighUnexpectedBehavior =
const hasNoSevereUnexpectedBehavior =
!scenarioResult.unexpectedBehaviors.some(
unexpectedBehavior =>
unexpectedBehavior.impact === 'HIGH'
unexpectedBehavior.impact === 'SEVERE'
);
const hasNoModerateUnexpectedBehavior =
!scenarioResult.unexpectedBehaviors.some(
Expand All @@ -45,11 +45,11 @@ const TestPlanResultsTable = ({
);
const passedAssertionsLength =
passedAssertions.length +
(hasNoHighUnexpectedBehavior ? 1 : 0) +
(hasNoSevereUnexpectedBehavior ? 1 : 0) +
(hasNoModerateUnexpectedBehavior ? 1 : 0);
const failedAssertionsLength =
failedAssertions.length +
(hasNoHighUnexpectedBehavior ? 0 : 1) +
(hasNoSevereUnexpectedBehavior ? 0 : 1) +
(hasNoModerateUnexpectedBehavior ? 0 : 1);

// Rows are sorted by priority descending, then result (failures then passes), then
Expand Down Expand Up @@ -89,9 +89,9 @@ const TestPlanResultsTable = ({
{
id: `UnexpectedBehavior_MUST_${nextId()}`,
assertion: {
text: 'Other behaviors that create high negative-impacts are not exhibited'
text: 'Other behaviors that create severe negative-impacts are not exhibited'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hyphen here isn't grammatically correct, it should be 'Other behaviors that create severe negative impacts are not exhibited'.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, although unsure if it was intentional. This could also be shortened in the future soon to:

Severe negative side effects do not occur

If you'd like to weigh in on that discussion.

},
passed: hasNoHighUnexpectedBehavior,
passed: hasNoSevereUnexpectedBehavior,
priorityString: 'MUST'
},
...optionalAssertionResults.map(e => ({
Expand Down Expand Up @@ -148,23 +148,33 @@ const TestPlanResultsTable = ({
)}
</tbody>
</Table>
Unexpected Behaviors:{' '}
Other behaviors that create negative impact:{' '}
{scenarioResult.unexpectedBehaviors.length ? (
<ul>
{scenarioResult.unexpectedBehaviors.map(
({ id, text, impact, details }) => {
const description = `${text} (Details: ${details}, Impact: ${impact})`;
return (
<li
key={id}
className="test-plan-results-li"
>
{description}
</li>
);
}
)}
</ul>
<Table
bordered
responsive
aria-label={`Undesirable behaviors for test ${test.title}`}
className={`test-plan-undesirable-behaviors-table ${tableClassName}`}
>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency, where possible can we call these unexpected behaviors?

<thead>
<tr>
<th>Behavior</th>
<th>Details</th>
<th>Impact</th>
</tr>
</thead>
<tbody>
{scenarioResult.unexpectedBehaviors.map(
({ id, text, details, impact }) => (
<tr key={id}>
<td>{text}</td>
<td>{details}</td>
<td>{impact}</td>
</tr>
)
)}
</tbody>
</Table>
) : (
'None'
)}
Expand Down
10 changes: 5 additions & 5 deletions client/components/common/TestPlanResultsTable/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// For each command, high and moderate negative behaviors is tracked, so 2
// For each command, severe and moderate negative behaviors is tracked, so 2
const NUMBER_NEGATIVE_IMPACTS = 2;

const getImpactFailedAssertionCount = (scenarioResults, impact) => {
Expand Down Expand Up @@ -28,10 +28,10 @@ export const calculateAssertionsCount = testResult => {
const maxUnexpectedBehaviorsPassCount =
testResult.scenarioResults.length * NUMBER_NEGATIVE_IMPACTS;

// Any high impact negative behavior is a failure
const highImpactFailedAssertionCount = getImpactFailedAssertionCount(
// Any severe impact negative behavior is a failure
const severeImpactFailedAssertionCount = getImpactFailedAssertionCount(
testResult.scenarioResults,
'HIGH'
'SEVERE'
);

// Any moderate impact negative behavior is a failure
Expand All @@ -41,7 +41,7 @@ export const calculateAssertionsCount = testResult => {
);

const weightedImpactFailedAssertionsCount =
highImpactFailedAssertionCount + moderateImpactFailedAssertionCount;
severeImpactFailedAssertionCount + moderateImpactFailedAssertionCount;

const weightedImpactPassedAssertionsCount =
maxUnexpectedBehaviorsPassCount - weightedImpactFailedAssertionsCount;
Expand Down
3 changes: 1 addition & 2 deletions client/resources/aria-at-harness.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,6 @@ function renderVirtualInstructionDocument(doc) {
value(failOption.description),
id(`${failOptionId}-${commandIndex}-checkbox`),
className([`undesirable-${commandIndex}`]),
tabIndex(failOption.tabbable ? '0' : '-1'),
disabled(!failOption.enabled),
checked(failOption.checked),
focus(failOption.focus),
Expand Down Expand Up @@ -488,7 +487,7 @@ function renderVirtualInstructionDocument(doc) {
id(`${failOptionId}-${commandIndex}-impact`),
ariaLabel(`Impact for ${failOption.description}`),
option(UnexpectedBehaviorImpactMap.MODERATE),
option(UnexpectedBehaviorImpactMap.HIGH),
option(UnexpectedBehaviorImpactMap.SEVERE),
disabled(!failOption.checked),
onchange(ev =>
failOption.impactchange(/** @type {HTMLInputElement} */ (ev.currentTarget).value)
Expand Down
4 changes: 2 additions & 2 deletions client/resources/aria-at-test-io-format.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -817,7 +817,7 @@ class BehaviorInput {
};
}

// { assertionId, priority, assertionStatement, assertionPhrase, refIds, tokenizedAssertionStatements }
// { assertionId, priority, assertionStatement, assertionPhrase, refIds, tokenizedAssertionStatements, tokenizedAssertionPhrases }
return {
priority: assertion.priority,
assertion:
Expand Down Expand Up @@ -1634,7 +1634,7 @@ const AssertionFailJSONMap = createEnumMap({

const UnexpectedBehaviorImpactMap = createEnumMap({
MODERATE: 'Moderate',
HIGH: 'High',
SEVERE: 'Severe',
});

/** @typedef {SubmitResultDetailsCommandsAssertionsPass | SubmitResultDetailsCommandsAssertionsFail} SubmitResultAssertionsJSON */
Expand Down
10 changes: 5 additions & 5 deletions client/resources/aria-at-test-run.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ export const AssertionResultMap = createEnumMap({

export const UnexpectedBehaviorImpactMap = createEnumMap({
MODERATE: 'Moderate',
HIGH: 'High',
SEVERE: 'Severe',
});

/**
Expand Down Expand Up @@ -841,9 +841,9 @@ function resultsTableDocument(state) {
...command.additionalAssertions,
];

let passingAssertions = ['No passing assertions.'];
let failingAssertions = ['No failing assertions.'];
let unexpectedBehaviors = ['No unexpected behaviors.'];
let passingAssertions = ['No passing assertions'];
let failingAssertions = ['No failing assertions'];
let unexpectedBehaviors = ['None'];

if (allAssertions.some(({ result }) => result === CommonResultMap.PASS)) {
passingAssertions = allAssertions
Expand Down Expand Up @@ -900,7 +900,7 @@ function resultsTableDocument(state) {
items: failingAssertions,
},
unexpectedBehaviors: {
description: 'Unexpected Behaviors',
description: 'Other behaviors that create negative impact:',
items: unexpectedBehaviors,
},
},
Expand Down
2 changes: 1 addition & 1 deletion server/graphql-schema.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -774,7 +774,7 @@ const graphqlSchema = gql`

enum UnexpectedBehaviorImpact {
MODERATE
HIGH
SEVERE
}

"""
Expand Down
2 changes: 1 addition & 1 deletion server/scripts/populate-test-data/populateFakeTestResults.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ const getFake = async ({
});
testResult.scenarioResults[0].unexpectedBehaviors.push({
id: 'OTHER',
impact: 'HIGH',
impact: 'SEVERE',
details: 'Seeded other unexpected behavior'
});
break;
Expand Down
18 changes: 9 additions & 9 deletions shared/getMetrics.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,21 +169,21 @@ const getMetrics = ({
const result = { scenarioResult, testResult, testPlanReport };

// Each command has 2 additional assertions:
// * Other behaviors that create high negative impact
// * Other behaviors that create severe negative impact
// * Other behaviors that create moderate negative impact
const commandsCount = countCommands({ ...result });

const highImpactFailedAssertionCount = countUnexpectedBehaviorsImpact(
const severeImpactFailedAssertionCount = countUnexpectedBehaviorsImpact(
{ ...result },
'HIGH'
'SEVERE'
);
const moderateImpactFailedAssertionCount = countUnexpectedBehaviorsImpact(
{ ...result },
'MODERATE'
);

const highImpactPassedAssertionCount =
commandsCount - highImpactFailedAssertionCount;
const severeImpactPassedAssertionCount =
commandsCount - severeImpactFailedAssertionCount;
const moderateImpactPassedAssertionCount =
commandsCount - moderateImpactFailedAssertionCount;

Expand All @@ -193,8 +193,8 @@ const getMetrics = ({
assertionsFailedCount: requiredAssertionsFailedCount
} = calculateAssertionPriorityCounts(result, 'REQUIRED');
requiredAssertionsCount += commandsCount;
requiredAssertionsPassedCount += highImpactPassedAssertionCount;
requiredAssertionsFailedCount += highImpactFailedAssertionCount;
requiredAssertionsPassedCount += severeImpactPassedAssertionCount;
requiredAssertionsFailedCount += severeImpactFailedAssertionCount;

let {
assertionsCount: optionalAssertionsCount,
Expand Down Expand Up @@ -262,8 +262,8 @@ const getMetrics = ({
testsCount,
testsFailedCount,
unexpectedBehaviorCount,
highImpactPassedAssertionCount,
highImpactFailedAssertionCount,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name highImpactFailedAssertionCount is still present in server/util/getMetrics.js is there a plan to remove it from there?

Copy link
Contributor Author

@howard-e howard-e Feb 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this comment may have been before the latest changes from main were merged in here, so this may be outdated? server/util/getMetrics.js isn't on this branch anymore.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right!

severeImpactPassedAssertionCount,
severeImpactFailedAssertionCount,
moderateImpactPassedAssertionCount,
moderateImpactFailedAssertionCount,
commandsCount,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since metrics are saved in the database does that mean we should have a migration to make sure the metrics are up-to-date?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this hasn't yet gone to the production environment yet to affect that database, no need to write that migration. There wouldn't be any metrics that include those properties just yet.

Expand Down
Loading