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 the rendering of errors for various controls. #424

Merged
merged 4 commits into from
Feb 17, 2018

Conversation

lcreid
Copy link
Contributor

@lcreid lcreid commented Feb 5, 2018

This pull request adds tests for various controls when there is an validation failure on the control. All these tests document the behaviour of the master branch as of the time the PR was created.

Bootstrap 4's handling of errors is different that Bootstrap 3's, and has led to some issues already, e.g. #417 and #395. The tests here, therefore, document current functionality, so they give us a baseline for any required changes. These tests should not be considered a declaration about how bootstrap_form should render errors with Bootstrap 4.

These tests also provide more coverage to ensure we don't introduce regressions as we address #417, #395 and any other issues that come up.

Given this PR contains only a few tests which don't significantly increase the run time of the test suite, I hope it's easy to merge. It would be great if someone other that @mattbrictson can review this PR, to take some of the load off Matt's shoulders.

@mattbrictson
Copy link
Contributor

Nice work. Always good to have additional test coverage where it makes sense. 💯

@mattbrictson mattbrictson merged commit fcb228a into bootstrap-ruby:master Feb 17, 2018
@lcreid lcreid deleted the error-tests branch February 17, 2018 21:52
lcreid added a commit to lcreid/rails-bootstrap-forms that referenced this pull request Jun 4, 2018
* Test selects error.

* Add tests for date, time, datetime selects.

* date, time, datetime tests pass with old implementation.

* Tests for file and collection selects.
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.

2 participants