Add tests for govuk-c-button component
#461
Conversation
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-review-pr-461
|
This PR includes duplicate testing for inputs based on an assumption that the forking elements could break 'attributes' 'classes' (etc) from being applied. I'm not sure if we need to be so strict. Comments welcome. |
NickColley
changed the title
govuk-c-button component
NickColley
force-pushed
the
add-tests-for-button-component
branch
from
1469f36
to
cff0629
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-review-pr-461
|
@36degrees I've reworded the tests so they read correctly, I'm not sure how to break them up as you described without losing track of the code coverage, if you want to do more than in this PR let's catch up and do it together. |
| }) | ||
|
|
||
| describe('implicit button element', () => { | ||
| it('renders a link if you pass an href', () => { |
There was a problem hiding this comment.
Context is 'implicit button element' but this is testing for an implicit link?
| const $component = $('.govuk-c-button') | ||
| expect($component.get(0).tagName).toEqual('input') | ||
| expect($component.attr('type')).toEqual('submit') | ||
| expect($component.val()).toContain('Save and continue') |
There was a problem hiding this comment.
I don't think this should be being expected as part of this test.
|
|
||
| const $component = $('.govuk-c-button') | ||
| expect($component.get(0).tagName).toEqual('a') | ||
| expect($component.attr('href')).toEqual('/') |
There was a problem hiding this comment.
This doesn't relate to the test description. I think we should just be testing that it's rendering a link.
|
|
||
| const $component = $('.govuk-c-button') | ||
| expect($component.get(0).tagName).toEqual('button') | ||
| expect($component.html()).toContain('Start <em>now</em>') |
There was a problem hiding this comment.
Again, I think we should just be testing that we render a button.
|
|
||
| describe('link', () => { | ||
| it('renders with anchor, href and an accessible role of button', () => { | ||
| const { $ } = render('button', examples.link) |
There was a problem hiding this comment.
The link example is an implicit link, because it specifies an href but not an element. So I think this is testing the same thing as the implicit link test on :223?
NickColley
force-pushed
the
add-tests-for-button-component
branch
from
cff0629
to
92026b9
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-review-pr-461
NickColley
force-pushed
the
add-tests-for-button-component
branch
from
92026b9
to
9a33c88
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-review-pr-461
|
@36degrees updated based on your comments |
|
One last thing - I need to add an entry to the changelog. |
We break this down into four groups (as represented by describe blocks). 1. Input buttons 2. Link buttons 3. Explicit buttons 4. Implicit buttons Also removes examples in `button.yaml` that are now covered by tests.
NickColley
force-pushed
the
add-tests-for-button-component
branch
from
9a33c88
to
580eb45
Compare
We break this down into four groups (as represented by describe blocks).
Also removes examples in
button.yamlthat are now covered by tests.As per https://trello.com/c/LzkwDRYe/639-automated-tests-for-button-component