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.

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

test(radio): Change tests to react-testing-library and improve coverage #381

Merged
merged 36 commits into from
Feb 6, 2020
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
4533ba9
test(radio): Switch ests to RTL and improve coverage
mannycarrera4 Dec 16, 2019
58b032d
test(radio): Add tests and convert them to RTL
mannycarrera4 Dec 18, 2019
1573fd9
fix(radio): Refactor tests for radio group
mannycarrera4 Jan 9, 2020
ea56b0d
test: Add cypress test for Radio
mannycarrera4 Jan 13, 2020
fb992c3
test: Re write test for unit
mannycarrera4 Jan 13, 2020
b8df3d0
fix(radio): Add states story
mannycarrera4 Jan 13, 2020
c544833
test(radio): Add test for radio group
mannycarrera4 Jan 17, 2020
e9ed794
test(radio): Remove unused const
mannycarrera4 Jan 17, 2020
71a588a
test(radio): Add radio group cypress test
mannycarrera4 Jan 17, 2020
42a3cd0
fix(radio): Update stories based on new static states table
mannycarrera4 Jan 21, 2020
6265720
fix(radio): Remove extra prop for story
mannycarrera4 Jan 21, 2020
568f2fa
test(text-input): Change tests to react-testing-library and improve c…
stephanerangaya Jan 24, 2020
3b0ddf6
fix(radio): Add visual story for radio group
mannycarrera4 Jan 27, 2020
3dfb60b
chore: Release v3.3.0 (#425)
anicholls Jan 28, 2020
e4151d1
test(switch): Redo switch tests in react-testing-library (#386)
lychyi Jan 28, 2020
7d8b8ea
chore: Prevent build on travis release (#431)
lychyi Jan 28, 2020
60efc8d
chore: Release v3.3.1 (#432)
lychyi Jan 29, 2020
8f25acb
test(radio): Add cypress helper to get by text
mannycarrera4 Jan 29, 2020
e5d8573
test(radio): Add cypress testing library
mannycarrera4 Jan 29, 2020
95f1380
test(radio): Remove cypress testing library
mannycarrera4 Jan 29, 2020
94dab81
fix(radio): Remove import for cypress testing
mannycarrera4 Jan 29, 2020
c4732d0
fix: Add missing labs-core imports (#435)
anicholls Jan 30, 2020
038174e
chore: Release v3.3.2 (#436)
lychyi Jan 30, 2020
a6de8cd
test(radio): Update test per comments, remove default cypress story
mannycarrera4 Jan 31, 2020
3469ac6
test(radio): Remove test and fix input ref test
mannycarrera4 Jan 31, 2020
17f4af2
test(radio): Fix disabled cypress test
mannycarrera4 Jan 31, 2020
92d1a57
test(radio): Update test to have right callback
mannycarrera4 Feb 1, 2020
a2dd012
fix: Fix merge conflicts
mannycarrera4 Feb 4, 2020
b7c6a63
fix: Update missed package.json and revert cypress tsconfig
mannycarrera4 Feb 4, 2020
65d1e74
test(radio): Fix test to test for onchange
mannycarrera4 Feb 5, 2020
2c9f3e1
Merge branch 'master' of git://github.com/Workday/canvas-kit into mc-…
mannycarrera4 Feb 6, 2020
bf1d921
test(radio): Remove cypress helpers, use cypress testing library
mannycarrera4 Feb 6, 2020
630cfe3
test(radio): Remove only test
mannycarrera4 Feb 6, 2020
648876b
Merge branch 'master' into mc-testing-radio
mannycarrera4 Feb 6, 2020
6fa683f
Merge branch 'master' into mc-testing-radio
mannycarrera4 Feb 6, 2020
cad4b83
Merge branch 'master' into mc-testing-radio
mannycarrera4 Feb 6, 2020
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
15 changes: 15 additions & 0 deletions cypress/helpers/dom.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
export const getByLabelText = (label: string) =>
function getByLabelText($container?: JQuery) {
const $label = ($container || Cypress.$('body')).find(`label:contains(${label})`);

if (!$label.length) {
throw Error(`Could not find a label containing "${label}"`);
}
const id = $label.attr('for');
try {
const $input = Cypress.$(`[id="${id}"]`);
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we worried that this doesn't account for wrapping labels?

i.e.

<label> 
  <input />
</label>

return $input;
} catch (e) {
throw Error(`Could not find an element labelled by "${label}`);
}
};
3 changes: 2 additions & 1 deletion cypress/helpers/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import * as dom from './dom';
import * as stories from './stories';
import * as modal from './modal';

export {modal, stories};
export {dom, modal, stories};
48 changes: 48 additions & 0 deletions cypress/integration/Radio.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import * as h from '../helpers';

const getRadio = () => {
return cy.get(`[type="radio"]`);
};

describe('Radio', () => {
before(() => {
h.stories.visit();
});

context(`given the Default story is rendered`, () => {
beforeEach(() => {
h.stories.load('Components|Inputs/Radio/React/Left Label/Radio', story);
});

it('should pass accessibility checks', () => {
cy.checkA11y();
});

context('when clicked', () => {
beforeEach(() => {
cy.getByLabelText('E-mail').click();
});

it.only('should be checked', () => {
getRadio().should('be.checked');
});
});
});

before(() => {
h.stories.visit();
});
context(`given the 'Disabled' story is rendered`, () => {
beforeEach(() => {
h.stories.load('Components|Inputs/Radio/React/Left Label/Radio', 'Disabled');
});

it('should pass accessibility checks', () => {
cy.checkA11y();
});

it('should be disabled', () => {
getRadio().should('be.disabled');
});
});
});
29 changes: 29 additions & 0 deletions cypress/integration/RadioGroup.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import * as h from '../helpers';

describe('Radio Group', () => {
before(() => {
h.stories.visit();
});
['Default', 'Alert', 'Error with Grow'].forEach(story => {
context(`given the '${story}' story is rendered`, () => {
beforeEach(() => {
h.stories.load('Components|Inputs/Radio/React/Top Label/Radio Group', story);
});

it('should pass accessibility checks', () => {
cy.checkA11y();
Copy link
Contributor

Choose a reason for hiding this comment

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

Random point of discussion: it really bugs me how much of a black box this is. For example, Radio group labels need to be legends - does this check that? Does this ensure aria-describedby is set when there's an error? I have no clue. In general, it makes the a11y specs really unclear. Not sure what the solution is without manually checking it all.

Copy link
Member

Choose a reason for hiding this comment

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

I asked Accessibility this question a few months ago. They estimate axe-core catches around 13% of accessibility issues. They think we should run it because it is so cheap to do so, but not treat it as a replacement for accessibility specifications.

What you describe should be specifications in our Cypress files.

describe('given the Error story is rendered', () => {
  it('should have a legend element', () => {
    // get radio input
    // .parents('legend').should('exist')
  })

  it('should have an aria-describedby linking to the input element', () => {
    // find radio input and get id to check against the value of the `aria-describedby`
  })
})

I think the test block description is misleading. It should say something like:

it('should pass axe-core checks', () => {

To clarify:
cy.checkA11y() is not a replacement for accessibility specifications. We simply treat it as a black box and not care if we duplicate accessibility checks.

});

context('when clicking one radio and then selecting another radio', () => {
beforeEach(() => {
cy.getByLabelText('E-mail').click();
cy.getByLabelText('Mail').click();
});

it('should one have one radio selected', () => {
cy.getByLabelText('Mail').should('be.checked');
});
});
});
});
});
1 change: 1 addition & 0 deletions cypress/support/commands.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as axe from 'axe-core';
import {Promise} from 'cypress/types/bluebird';
import './query';

declare global {
interface Window {
Expand Down
1 change: 1 addition & 0 deletions cypress/support/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ import 'cypress-axe';
import 'cypress-storybook/cypress';

import './commands';
import './query';
63 changes: 63 additions & 0 deletions cypress/support/query.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import * as dom from '../helpers/dom';

declare global {
namespace Cypress {
interface Chainable<Subject> {
getByLabelText(label: string): Cypress.Chainable<JQuery>;
}
}
}

const createRetryableCommand = (
name: string,
message: string,
options,
getValue: (...args: any[]) => any
) => {
Cypress._.defaults(options, {
log: true,
timeout: 4000,
});

if (options.log) {
options._log = Cypress.log({
message: [message],
});
}

const retryValue = () => {
return Cypress.Promise.try(getValue).catch(err => {
options.error = err;
return (cy as any).retry(retryValue, options);
});
};

const resolveValue = () => {
return Cypress.Promise.try(retryValue).then(value => {
return (cy as any).verifyUpcomingAssertions(value, options, {
onRetry: resolveValue,
});
});
};

return resolveValue().then(value => {
options._log.set('$el', value);
if (options._log) {
options._log.snapshot().end();
}

return value;
});
};

Cypress.Commands.add(
'getByLabelText',
{prevSubject: ['optional', 'element']},
($container: JQuery | undefined, labelText: string, options = {}) => {
return createRetryableCommand('getByLabelText', labelText, options, () => {
return dom.getByLabelText(labelText)($container);
});
}
);

// We'll be adding commands to this file similar to Cypress testing Library
4 changes: 3 additions & 1 deletion cypress/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
{
"compilerOptions": {
"types": ["cypress"]
"types": ["cypress", "@types/testing-library__cypress"]
mannycarrera4 marked this conversation as resolved.
Show resolved Hide resolved
},
"strict": true,
"include": ["**/*.ts"],
"exclude": []
}
3 changes: 0 additions & 3 deletions modules/_labs/core/react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,5 @@
"emotion-theming": "^10.0.10",
"lodash": "^4.17.14",
"rtl-css-js": "^1.13.1"
},
"devDependencies": {
"@workday/canvas-system-icons-web": "^1.0.20"
}
}
1 change: 0 additions & 1 deletion modules/_labs/header/react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
"@emotion/core": "^10.0.21",
"@emotion/styled": "^10.0.17",
"@workday/canvas-kit-labs-react-combobox": "^3.3.2",
"@workday/canvas-kit-labs-react-menu": "^3.3.0",
mannycarrera4 marked this conversation as resolved.
Show resolved Hide resolved
"@workday/canvas-kit-react-button": "^3.3.0",
"@workday/canvas-kit-react-common": "^3.3.0",
"@workday/canvas-kit-react-core": "^3.2.0",
Expand Down
6 changes: 6 additions & 0 deletions modules/form-field/react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,5 +45,11 @@
"@workday/canvas-kit-react-common": "^3.3.0",
"@workday/canvas-kit-react-core": "^3.2.0",
"uuid": "^3.3.3"
},
"devDependencies": {
"@workday/canvas-kit-react-checkbox": "^3.3.2",
"@workday/canvas-kit-react-radio": "^3.3.2",
"@workday/canvas-kit-react-select": "^3.3.2",
"@workday/canvas-kit-react-text-input": "^3.3.2"
}
}
93 changes: 90 additions & 3 deletions modules/form-field/react/stories/stories_Radio.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@
import * as React from 'react';
import {storiesOf} from '@storybook/react';
import withReadme from 'storybook-readme/with-readme';
import {ControlledComponentWrapper} from '../../../../utils/storybook';
import {StaticStates} from '@workday/canvas-kit-labs-react-core/lib/StaticStates';
import {
ControlledComponentWrapper,
ComponentStatesTable,
permutateProps,
} from '../../../../utils/storybook';

import {Radio, RadioGroup} from '../../../radio/react';
import FormField from '../index';
Expand Down Expand Up @@ -72,7 +77,7 @@ storiesOf('Components|Inputs/Radio/React/Top Label/Radio', module)
<ControlledComponentWrapper
controlledProp={ControlledComponentWrapper.ControlledProp.Checked}
>
<Radio id="1" value="email" label="E-mail" />
<Radio value="email" label="E-mail" />
</ControlledComponentWrapper>
</FormField>
));
Expand Down Expand Up @@ -140,7 +145,89 @@ storiesOf('Components|Inputs/Radio/React/Left Label/Radio', module)
<ControlledComponentWrapper
controlledProp={ControlledComponentWrapper.ControlledProp.Checked}
>
<Radio id="1" value="email" label="E-mail" />
<Radio value="email" label="E-mail" />
</ControlledComponentWrapper>
</FormField>
))
.add('Disabled', () => (
<FormField label="Label" labelPosition={FormField.LabelPosition.Left}>
<ControlledComponentWrapper
controlledProp={ControlledComponentWrapper.ControlledProp.Checked}
>
<Radio disabled={true} value="email" label="E-mail" />
</ControlledComponentWrapper>
</FormField>
));

storiesOf('Components|Inputs/Radio/React/Visual', module)
.addParameters({component: Radio})
.addDecorator(withReadme(README))
.add('States', () => (
<div>
<h3>Radio</h3>
<StaticStates>
<ComponentStatesTable
rowProps={permutateProps({
checked: [{value: true, label: 'Checked'}, {value: false, label: 'Unchecked'}],
})}
columnProps={permutateProps(
{
className: [
{label: 'Default', value: ''},
{label: 'Hover', value: 'hover'},
{label: 'Focus', value: 'focus'},
{label: 'Focus Hover', value: 'focus hover'},
{label: 'Active', value: 'active'},
{label: 'Active Hover', value: 'active hover'},
],
disabled: [{label: '', value: false}, {label: 'Disabled', value: true}],
},
props => {
if (props.disabled && !['', 'hover'].includes(props.className)) {
return false;
}
return true;
}
)}
>
{props => (
<Radio
{...props}
onChange={() => {}} // eslint-disable-line no-empty-function
label="Radio"
/>
)}
</ComponentStatesTable>
</StaticStates>
<h3>Radio Group</h3>
<StaticStates>
<ComponentStatesTable
rowProps={permutateProps({
error: [
{value: undefined, label: 'No Error'},
{value: FormField.ErrorType.Alert, label: 'Alert'},
{value: FormField.ErrorType.Error, label: 'Error'},
],
})}
columnProps={[{label: 'Default', props: {}}]}
>
{props => (
<FormField
useFieldset={true}
hintText={props.error ? hintText : undefined}
hintId={hintId}
labelPosition={FormField.LabelPosition.Left}
{...props}
>
<ControlledComponentWrapper>
<RadioGroup name="contact">
<Radio id="1" value="email" label="E-mail" />
mannycarrera4 marked this conversation as resolved.
Show resolved Hide resolved
<Radio id="2" value="fax" label="Fax (disabled)" disabled={true} />
</RadioGroup>
</ControlledComponentWrapper>
</FormField>
)}
</ComponentStatesTable>
</StaticStates>
</div>
));
4 changes: 3 additions & 1 deletion modules/page-header/react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@
"@emotion/core": "^10.0.21",
"@emotion/styled": "^10.0.17",
"@workday/canvas-kit-react-button": "^3.3.0",
"@workday/canvas-kit-react-core": "^3.2.0"
"@workday/canvas-kit-react-common": "^3.3.0",
"@workday/canvas-kit-react-core": "^3.2.0",
"@workday/canvas-kit-react-icon": "^3.3.0"
},
"devDependencies": {
"@workday/canvas-system-icons-web": "^1.0.20"
Expand Down
Loading