Skip to content

Commit

Permalink
feat(manual-auto-pass): add auto-pass for CSS content and CSS positio…
Browse files Browse the repository at this point in the history
…ning (#2687)

#### Description of changes

Per discussion in #2644, the new manual auto-pass behavior we added for some of the Landmarks requirements would also be nice to have for the Semantics > CSS content and Sequence > CSS positioning requirements when their visualization analyzers find no relevant elements.

This PR generalizes the "auto-pass if no landmarks" behavior to a "auto-pass if no results relevant to the requirement" behavior and adds it in to the 2 CSS requirements. To do this, it updates the assessment store to only pass instance data with results relevant to the current requirement when it calls a requirement's getInitialManualTestStatus function.

I considered an alternative approach of leaving assessment store as-is and creating a factory function for each requirement to specify its own `autoPassIfNoResultsFor(key)` rather than a single `autoPassIfNoResults`; I liked that because it avoided adding more complexity to `assessment-store`, which is already a very complex class, but I felt that it was better for data encapsulation if requirements were systematically prevented from seeing/depending on other requirements' data, and I thought the encapsulation was more valuable.

![gif of 2 CSS requirements auto-passing on a page with no matching instances](https://user-images.githubusercontent.com/376284/81997746-89da2800-9605-11ea-8617-787a6b4b3f1b.gif)

#### Pull request checklist
<!-- If a checklist item is not applicable to this change, write "n/a" in the checkbox -->
- [n/a] Addresses an existing issue: #0000
- [x] Ran `yarn fastpass`
- [x] Added/updated relevant unit test(s) (and ran `yarn test`)
- [x] Verified code coverage for the changes made. Check coverage report at: `<rootDir>/test-results/unit/coverage`
- [x] PR title *AND* final merge commit title both start with a semantic tag (`fix:`, `chore:`, `feat(feature-name):`, `refactor:`). See `CONTRIBUTING.md`.
- [x] (UI changes only) Added screenshots/GIFs to description above
- [x] (UI changes only) Verified usability with NVDA/JAWS
  • Loading branch information
dbjorge authored May 15, 2020
1 parent dd08300 commit 4df86fe
Show file tree
Hide file tree
Showing 10 changed files with 94 additions and 65 deletions.
9 changes: 9 additions & 0 deletions src/assessments/auto-pass-if-no-results.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.
import { ManualTestStatus } from 'common/types/manual-test-status';
import { InstanceIdToInstanceDataMap } from 'common/types/store-data/assessment-result-data';
import { isEmpty } from 'lodash';

export function autoPassIfNoResults(instanceData: InstanceIdToInstanceDataMap): ManualTestStatus {
return isEmpty(instanceData) ? ManualTestStatus.PASS : ManualTestStatus.UNKNOWN;
}
14 changes: 0 additions & 14 deletions src/assessments/landmarks/auto-pass-if-no-landmarks.ts

This file was deleted.

4 changes: 2 additions & 2 deletions src/assessments/landmarks/test-steps/no-repeating-content.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ import { VisualizationType } from 'common/types/visualization-type';
import { link } from 'content/link';
import * as content from 'content/test/landmarks/no-repeating-content';
import { AssessmentVisualizationEnabledToggle } from 'DetailsView/components/assessment-visualization-enabled-toggle';
import { autoPassIfNoResults } from '../../auto-pass-if-no-results';
import { AnalyzerConfigurationFactory } from '../../common/analyzer-configuration-factory';
import { ManualTestRecordYourResults } from '../../common/manual-test-record-your-results';
import * as Markup from '../../markup';
import { Requirement } from '../../types/requirement';
import { autoPassIfNoLandmarks } from '../auto-pass-if-no-landmarks';
import { LandmarkTestStep } from './test-steps';

const description: JSX.Element = (
Expand Down Expand Up @@ -66,7 +66,7 @@ export const NoRepeatingContent: Requirement = {
description,
howToTest,
isManual: true,
getInitialManualTestStatus: autoPassIfNoLandmarks,
getInitialManualTestStatus: autoPassIfNoResults,
isVisualizationSupportedForResult: doesResultHaveMainRole,
guidanceLinks: [link.WCAG_1_3_1, link.WCAG_2_4_1],
getAnalyzer: provider =>
Expand Down
4 changes: 2 additions & 2 deletions src/assessments/landmarks/test-steps/primary-content.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ import { VisualizationType } from 'common/types/visualization-type';
import { link } from 'content/link';
import * as content from 'content/test/landmarks/primary-content';
import { AssessmentVisualizationEnabledToggle } from 'DetailsView/components/assessment-visualization-enabled-toggle';
import { autoPassIfNoResults } from '../../auto-pass-if-no-results';
import { AnalyzerConfigurationFactory } from '../../common/analyzer-configuration-factory';
import { ManualTestRecordYourResults } from '../../common/manual-test-record-your-results';
import * as Markup from '../../markup';
import { Requirement } from '../../types/requirement';
import { autoPassIfNoLandmarks } from '../auto-pass-if-no-landmarks';
import { LandmarkTestStep } from './test-steps';

const description: JSX.Element = (
Expand Down Expand Up @@ -65,7 +65,7 @@ export const PrimaryContent: Requirement = {
description,
howToTest,
isManual: true,
getInitialManualTestStatus: autoPassIfNoLandmarks,
getInitialManualTestStatus: autoPassIfNoResults,
isVisualizationSupportedForResult: doesResultHaveMainRole,
guidanceLinks: [link.WCAG_1_3_1, link.WCAG_2_4_1],
getAnalyzer: provider =>
Expand Down
6 changes: 6 additions & 0 deletions src/assessments/semantics/test-steps/css-content.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.
import { autoPassIfNoResults } from 'assessments/auto-pass-if-no-results';
import { NewTabLink } from 'common/components/new-tab-link';
import { VisualizationType } from 'common/types/visualization-type';
import { link } from 'content/link';
import { TestAutomaticallyPassedNotice } from 'content/test/common/test-automatically-passed-notice';
import * as content from 'content/test/semantics/css-content';
import { AssessmentVisualizationEnabledToggle } from 'DetailsView/components/assessment-visualization-enabled-toggle';
import * as React from 'react';
Expand All @@ -17,6 +19,9 @@ const cssContentHowToTest: JSX.Element = (
<p>
The visual helper for this requirement highlights content inserted in the page using CSS{' '}
<Markup.CodeTerm>:before</Markup.CodeTerm> or <Markup.CodeTerm>:after</Markup.CodeTerm>.
</p>
<TestAutomaticallyPassedNotice />
<p>
This procedure uses the{' '}
<NewTabLink href="https://chrome.google.com/webstore/detail/web-developer/bfbameneiokkgbdmiekhjnmfkcnldhhm">
Web Developer
Expand Down Expand Up @@ -101,6 +106,7 @@ export const CssContent: Requirement = {
description: cssContentDescription,
howToTest: cssContentHowToTest,
isManual: true,
getInitialManualTestStatus: autoPassIfNoResults,
guidanceLinks: [link.WCAG_1_3_1],
...content,
getAnalyzer: provider =>
Expand Down
4 changes: 4 additions & 0 deletions src/assessments/sequence/test-steps/css-positioning.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.
import { autoPassIfNoResults } from 'assessments/auto-pass-if-no-results';
import { NewTabLink } from 'common/components/new-tab-link';
import { VisualizationType } from 'common/types/visualization-type';
import { link } from 'content/link';
import { TestAutomaticallyPassedNotice } from 'content/test/common/test-automatically-passed-notice';
import * as content from 'content/test/sequence/css-positioning';
import { AssessmentVisualizationEnabledToggle } from 'DetailsView/components/assessment-visualization-enabled-toggle';
import * as React from 'react';
Expand All @@ -26,6 +28,7 @@ const howToTest: JSX.Element = (
CSS <Markup.Term>position:absolute</Markup.Term> or{' '}
<Markup.Term>float:right</Markup.Term>.
</p>
<TestAutomaticallyPassedNotice />
<p>
This procedure also uses the{' '}
<NewTabLink href="https://chrome.google.com/webstore/detail/web-developer/bfbameneiokkgbdmiekhjnmfkcnldhhm">
Expand Down Expand Up @@ -69,6 +72,7 @@ export const CssPositioning: Requirement = {
description: description,
howToTest: howToTest,
isManual: true,
getInitialManualTestStatus: autoPassIfNoResults,
...content,
guidanceLinks: [link.WCAG_1_3_2],
getAnalyzer: provider =>
Expand Down
11 changes: 8 additions & 3 deletions src/background/stores/assessment-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {
ScanCompletedPayload,
ScanUpdatePayload,
} from 'injected/analyzers/analyzer';
import { forEach, isEmpty } from 'lodash';
import { forEach, isEmpty, pickBy } from 'lodash';
import { DictionaryStringTo } from 'types/common-types';
import { AddResultDescriptionPayload, SelectTestSubviewPayload } from '../actions/action-payloads';
import { AssessmentDataConverter } from '../assessment-data-converter';
Expand Down Expand Up @@ -460,8 +460,13 @@ export class AssessmentStore extends BaseStoreImpl<AssessmentStoreData> {
return; // Never override an explicitly set status
}

const instanceMap = assessmentData.generatedAssessmentInstancesMap;
const status = getInitialManualTestStatus(instanceMap);
const allInstances = assessmentData.generatedAssessmentInstancesMap;
const instancesWithResultsForTestStep = pickBy(
allInstances,
(value, key) => value.testStepResults[testStepName] != null,
);

const status = getInitialManualTestStatus(instancesWithResultsForTestStep);
assessmentData.manualTestStepResultMap[testStepName].status = status;
this.updateManualTestStepStatus(assessmentData, testStepName, testType);
}
Expand Down
32 changes: 32 additions & 0 deletions src/tests/unit/tests/assessments/auto-pass-if-no-results.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.
import { autoPassIfNoResults } from 'assessments/auto-pass-if-no-results';
import { ManualTestStatus } from 'common/types/manual-test-status';
import { InstanceIdToInstanceDataMap } from 'common/types/store-data/assessment-result-data';

describe('autoPassIfNoResults', () => {
it('returns UNKNOWN for instance data with a result', () => {
const inputWithResult: InstanceIdToInstanceDataMap = {
'#some-element': {
target: ['#some-element'],
html: '<div id="some-element" />',
propertyBag: {
someDataProperty: 'some data',
},
testStepResults: {
'test-step-1': {
someResultData: 'some result data',
},
},
},
};

expect(autoPassIfNoResults(inputWithResult)).toBe(ManualTestStatus.UNKNOWN);
});

it('returns PASS for instance data with no results', () => {
const inputWithNoResults: InstanceIdToInstanceDataMap = {};

expect(autoPassIfNoResults(inputWithNoResults)).toBe(ManualTestStatus.PASS);
});
});

This file was deleted.

35 changes: 31 additions & 4 deletions src/tests/unit/tests/background/stores/assessment-store.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import {
AssessmentData,
AssessmentStoreData,
GeneratedAssessmentInstance,
InstanceIdToInstanceDataMap,
ManualTestStepResult,
PersistedTabInfo,
TestStepResult,
Expand Down Expand Up @@ -581,16 +582,40 @@ describe('AssessmentStore', () => {
scanIncompleteWarnings: [],
};

const expectedInstanceMap = {};
const fullInstanceMap: InstanceIdToInstanceDataMap = {
'#selector-1': {
target: ['#selector-1'],
html: '<div id="selector-1" />',
testStepResults: { 'assessment-1-step-1': { resultData: 'result data' } },
},
'#selector-2': {
target: ['#selector-2'],
html: '<div id="selector-2" />',
testStepResults: { 'assessment-1-step-2': { resultData: 'result data' } },
},
};
const instanceMapFilteredForTestStep1 = {
'#selector-1': fullInstanceMap['#selector-1'],
};

const mockGetInitialManualTestStatus = Mock.ofInstance(
(_: InstanceIdToInstanceDataMap) => ManualTestStatus.FAIL,
MockBehavior.Strict,
);
mockGetInitialManualTestStatus
.setup(m => m(instanceMapFilteredForTestStep1))
.returns(() => ManualTestStatus.FAIL)
.verifiable();

const stepMapStub = assessmentsProvider.getStepMap(assessmentType);
const stepConfig: Readonly<Requirement> = {
...assessmentsProvider.getStep(assessmentType, 'assessment-1-step-1'),
isManual: true,
getInitialManualTestStatus: () => ManualTestStatus.FAIL,
getInitialManualTestStatus: mockGetInitialManualTestStatus.object,
};

const assessmentData = new AssessmentDataBuilder()
.with('generatedAssessmentInstancesMap', expectedInstanceMap)
.with('generatedAssessmentInstancesMap', fullInstanceMap)
.with('testStepStatus', {
// should FAIL based on getInitialManualTestStatus
['assessment-1-step-1']: generateTestStepData(ManualTestStatus.FAIL, true),
Expand Down Expand Up @@ -643,11 +668,13 @@ describe('AssessmentStore', () => {
stepConfig.isVisualizationSupportedForResult,
),
)
.returns(() => expectedInstanceMap);
.returns(() => fullInstanceMap);

createStoreTesterForAssessmentActions('scanCompleted')
.withActionParam(payload)
.testListenerToBeCalledOnce(initialState, finalState);

mockGetInitialManualTestStatus.verifyAll();
});

test('onScanCompleted with a manual requirement skips getInitialManualTestStatus for requirements that already have a status', () => {
Expand Down

0 comments on commit 4df86fe

Please sign in to comment.