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

Add tests for error summary component #489

Merged
merged 2 commits into from
Feb 1, 2018
Merged

Conversation

kr8n3r
Copy link

@kr8n3r kr8n3r commented Feb 1, 2018

This PR:

  • Adds tests to error-summary component to ensure the markup is rendered correctly when providing arguments to the macro.
  • Updates CHANGELOG.MD.

Trello Card: https://trello.com/c/1PnIu9nh/644-automated-tests-for-error-summary-component

Tests to test macro output
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-489 February 1, 2018 10:03 Inactive
Copy link
Contributor

@alex-ju alex-ju left a comment

Choose a reason for hiding this comment

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

Looks good to me


it('allows description text to be passed whilst escaping HTML entities', () => {
const { $ } = render('error-summary', {
descriptionText: 'See errors below (▼)'
Copy link
Contributor

Choose a reason for hiding this comment

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

nice one

@kr8n3r kr8n3r merged commit a045094 into master Feb 1, 2018
@kr8n3r kr8n3r deleted the tests-for-error-summary branch February 1, 2018 11:28
})

const $component = $('.govuk-c-error-summary')
expect($component.hasClass('extra-class one-more-class')).toBeTruthy()
Copy link
Member

Choose a reason for hiding this comment

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

I am surprised this works - it seems to go against Cheerio and jQuery's documentation which both expect a single class to be passed. Wouldn't be surprised if this is a bug in Cheerio…

@alex-ju alex-ju mentioned this pull request Feb 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants