-
Notifications
You must be signed in to change notification settings - Fork 20
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
Refactor Address input component test file to new format #3316
base: main
Are you sure you want to change the base?
Refactor Address input component test file to new format #3316
Conversation
✅ Deploy Preview for ons-design-system-preview ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
I think part of the work on this was to move the examples like EXAMPLE_MANUAL_INPUT_FIELDS out into its own file and then be imported. Have a look at one of the ones Bali has done |
a2e0924
to
496bd95
Compare
Co-authored-by: rmccar <[email protected]>
Co-authored-by: rmccar <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a difficult one because I think the macro options file is a bit unwieldy - I think this one will need a pretty major revamp from the original test file.
As there are a lot of required params - i would probably start with a "GIVEN: Params: required" and then do when/then's for each of those params
After that I would go through each macro option in order and do its given/when/then. if there's a group that makes sense to go together, then by all means group those, and just make sure it's obvious what params are being covered.
}); | ||
}); | ||
|
||
it('renders manualLinkText` when provided with a default value for `manualLink`', () => { | ||
describe('WHEN: provided with a default value for manualLink', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
manualLink should be a given with two whens underneath that cover default and supplied values
Co-authored-by: rmccar <[email protected]>
Co-authored-by: rmccar <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of minor things
expect(results).toHaveNoViolations(); | ||
describe('FOR: Macro: Address-input', () => { | ||
describe('GIVEN: Params: required', () => { | ||
describe('WHEN: all required params are provided', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
describe('WHEN: all required params are provided', () => { | |
describe('WHEN: required params are provided', () => { |
value: '12345 The Road', | ||
width: '20@m', | ||
error: { text: 'Server error: address line 2' }, | ||
test('THEN: it renders autosuggest component with expected parameters', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this should be "THEN: the provided attributes are passed through to the autosuggest component" because that is what it is testing
What is the context of this PR?
Fixes: ONSdigital/design-team#132
This PR involves refactoring of the address input test document to follow the Given, When, Then testing approach.
How to review this PR
Functionality:
Does this testing refactor meet the following success criteria:
Look at the refactored code in relation to:
Checklist
This needs to be completed by the person raising the PR.