Testing multiple versions of Rails #278

koenpunt · 2016-07-05T22:08:02Z

This fixes #273

koenpunt · 2016-07-05T22:16:33Z

~~And I'm not sure why the Travis build isn't being triggered for this PR..?~~ it is, but it doesn't update here on GitHub: https://travis-ci.org/bootstrap-ruby/rails-bootstrap-forms/builds/142603928

mattbrictson · 2016-07-06T15:31:59Z

Thanks for taking a first shot at this. I'll try to review later today.

I would like to sort out the Travis issue, but unfortunately I'm not a project owner, just a contributor. That means I cannot see or edit the Travis config.

@potenza @carloslopes Can you help us turn on Travis for PRs?

mattbrictson · 2016-07-07T17:59:00Z

This is an excellent start! I like the use of the appraisal gem, and it is hooked up nicely with Travis.

Clearly we have some work to do in order to get tests passing on 4.1+.

My concerns so far are:

How do we do this in a piecemeal fashion so that we can merge into master sooner rather than later, but at the same time keep Travis green? Maybe work on getting 4.0 and 4.1 passing first, merge, then fix 4.2 and 5.0 in a separate PR?
The output from rails-dom-testing when an assertion fails is very hard to interpret (it seems to just call #inspect on the DOM nodes). Can we do better in terms of the output for failing DOM assertions?

 25) Failure:
BootstrapFormTest#test_the_field_is_wrapped_with_div.field_with_errors_when_form_for_is_used [/home/travis/build/bootstrap-ruby/rails-bootstrap-forms/test/bootstrap_form_test.rb:175]:
--- expected
+++ actual
@@ -1 +1 @@
-#<HTML::Node:0xXXXXXX @parent=nil, @children=[#<HTML::Tag:0xXXXXXX @parent=#<HTML::Node:0xXXXXXX ...>, @children=[#<HTML::Tag:0xXXXXXX @parent=#<HTML::Tag:0xXXXXXX ...>, @children=[#<HTML::Tag:0xXXXXXX @parent=#<HTML::Tag:0xXXXXXX ...>, @children=[], @line=1, @position=137, @name="input", @attributes={"name"=>"utf8", "type"=>"hidden", "value"=>"&#x2713;"}, @closing=:self>], @line=1, @position=90, @name="div", @attributes={"style"=>"margin:0;padding:0;display:inline"}, @closing=nil>, #<HTML::Tag:0xXXXXXX @parent=#<HTML::Tag:0xXXXXXX ...>, @children=[#<HTML::Tag:0xXXXXXX @parent=#<HTML::Tag:0xXXXXXX ...>, @children=[#<HTML::Tag:0xXXXXXX @parent=#<HTML::Tag:0xXXXXXX ...>, @children=[#<HTML::Text:0xXXXXXX @parent=#<HTML::Tag:0xXXXXXX ...>, @children=[], @line=1, @position=315, @content="Email">], @line=1, @position=260, @name="label", @attributes={"class"=>"control-label required", "for"=>"user_email"}, @closing=nil>], @line=1, @position=229, @name="div", @attributes={"class"=>"field_with_errors"}, @closing=nil>, #<HTML::Tag:0xXXXXXX @parent=#<HTML::Tag:0xXXXXXX ...>, @children=[#<HTML::Tag:0xXXXXXX @parent=#<HTML::Tag:0xXXXXXX ...>, @children=[], @line=1, @position=365, @name="input", @attributes={"class"=>"form-control", "id"=>"user_email", "name"=>"user[email]", "type"=>"text"}, @closing=:self>], @line=1, @position=334, @name="div", @attributes={"class"=>"field_with_errors"}, @closing=nil>, #<HTML::Tag:0xXXXXXX @parent=#<HTML::Tag:0xXXXXXX ...>, @children=[#<HTML::Text:0xXXXXXX @parent=#<HTML::Tag:0xXXXXXX ...>, @children=[], @line=1, @position=473, @content="can&#39;t be blank, is too short (minimum is 5 characters)">], @line=1, @position=448, @name="span", @attributes={"class"=>"help-block"}, @closing=nil>], @line=1, @position=195, @name="div", @attributes={"class"=>"form-group has-error"}, @closing=nil>], @line=1, @position=0, @name="form", @attributes={"accept-charset"=>"UTF-8", "action"=>"/users", "class"=>"new_user", "id"=>"new_user", "method"=>"post"}, @closing=nil>], @line=0, @position=0>
+#<HTML::Node:0xXXXXXX @parent=nil, @children=[#<HTML::Tag:0xXXXXXX @parent=#<HTML::Node:0xXXXXXX ...>, @children=[#<HTML::Tag:0xXXXXXX @parent=#<HTML::Tag:0xXXXXXX ...>, @children=[#<HTML::Tag:0xXXXXXX @parent=#<HTML::Tag:0xXXXXXX ...>, @children=[], @line=1, @position=116, @name="input", @attributes={"name"=>"utf8", "type"=>"hidden", "value"=>"&#x2713;"}, @closing=:self>], @line=1, @position=90, @name="div", @attributes={"style"=>"display:none"}, @closing=nil>, #<HTML::Tag:0xXXXXXX @parent=#<HTML::Tag:0xXXXXXX ...>, @children=[#<HTML::Tag:0xXXXXXX @parent=#<HTML::Tag:0xXXXXXX ...>, @children=[#<HTML::Tag:0xXXXXXX @parent=#<HTML::Tag:0xXXXXXX ...>, @children=[#<HTML::Text:0xXXXXXX @parent=#<HTML::Tag:0xXXXXXX ...>, @children=[], @line=1, @position=294, @content="Your e-mail address">], @line=1, @position=239, @name="label", @attributes={"class"=>"control-label required", "for"=>"user_email"}, @closing=nil>], @line=1, @position=208, @name="div", @attributes={"class"=>"field_with_errors"}, @closing=nil>, #<HTML::Tag:0xXXXXXX @parent=#<HTML::Tag:0xXXXXXX ...>, @children=[#<HTML::Tag:0xXXXXXX @parent=#<HTML::Tag:0xXXXXXX ...>, @children=[], @line=1, @position=358, @name="input", @attributes={"class"=>"form-control", "id"=>"user_email", "name"=>"user[email]", "type"=>"text"}, @closing=:self>], @line=1, @position=327, @name="div", @attributes={"class"=>"field_with_errors"}, @closing=nil>, #<HTML::Tag:0xXXXXXX @parent=#<HTML::Tag:0xXXXXXX ...>, @children=[#<HTML::Text:0xXXXXXX @parent=#<HTML::Tag:0xXXXXXX ...>, @children=[], @line=1, @position=466, @content="can&#39;t be blank, is too short (minimum is 5 characters)">], @line=1, @position=441, @name="span", @attributes={"class"=>"help-block"}, @closing=nil>], @line=1, @position=174, @name="div", @attributes={"class"=>"form-group has-error"}, @closing=nil>], @line=1, @position=0, @name="form", @attributes={"accept-charset"=>"UTF-8", "action"=>"/users", "class"=>"new_user", "id"=>"new_user", "method"=>"post"}, @closing=nil>], @line=0, @position=0>

koenpunt · 2016-07-07T18:02:40Z

We might be better of with a custom parser, which only checks a limited set of attributes. Because I already see issues with certain versions of Rails outputting different attributes and additional elements than others.

mattbrictson · 2016-07-07T22:32:57Z

We might be better of with a custom parser

Rather than risk reinventing the wheel, maybe we can continue using rails-dom-parser, but instead of assert_dom_equal, use assert_select and test for certain CSS expressions?

BTW, thanks for taking the lead on this. If the scope of the work gets overwhelming and you want to split it up somehow, let me know and I'll do my best to help out.

mattbrictson · 2016-07-07T22:40:02Z

Also: I think Travis integration is working now. Next time you push commits to this PR the Travis build status should get hooked up.

koenpunt · 2016-07-11T11:59:39Z

Cool, will see if I can squeeze in a update somewhere this week

I added an `assert_equivalent_xml` ActionView::TestCase, and then I changed all the `assert_equal` to `assert_equivalent_xml`.

this to prevent differences in defaults between rails versions

as of rails 4.2 the utf8 hidden field is no longer wrapped in a div

for some reason EquivalentXml can't handle attributes with dashes, so a workaround is provided.

since rails 5 the type for a datetime field is `datetime-local` instead of `datetime`

koenpunt · 2016-12-09T16:28:00Z

@mattbrictson I finally got around fixing the tests (with the equivalent-xml gem proposed by @lcreid).

I can imagine that certain logic that is now part of assert_equivalent_xml should be added to specific tests instead, but please have a look and let me know what you think :)

mattbrictson

Excellent! This looks great. I had one comment about reorganizing require statements. Otherwise, could you add a bit of documentation? Then I think this will be good to merge.

Explain in the README how to run appraisals.
Mention in the CHANGELOG that the project is now tested against multiple versions of Rails (even though this is not a user-facing feature or bug fix, I think users will be excited to hear it).

Thanks!

mattbrictson · 2016-12-09T19:11:13Z

test/bootstrap_radio_button_test.rb

@@ -1,4 +1,6 @@
 require 'test_helper'
+require 'nokogiri'
+require 'equivalent-xml'


I think these require statements should move to test_helper.rb?

The only reason those require statements are there is that's the file I arbitrarily chose to initially do the work. What I checked in yesterday I hadn't even reviewed myself. I didn't realize how quickly you were going to take the commit. :-) I agree the requires should go in test_helper.rb.

mattbrictson · 2016-12-09T19:19:52Z

@lcreid I tried to invite you as a reviewer to this PR, but apparently GH only allows maintainers (?) to be reviewers. In any case, I would appreciate your 2¢ before merging this if you have time. Thanks!

lcreid · 2016-12-09T20:35:48Z

@mattbrictson it looks good to me. @koenpunt obviously knows this issue inside out, to be able to add the special cases so quickly. Let me know if I can help out in any other way.

koenpunt · 2016-12-09T22:00:33Z

I've updated per the requested changes. We might consider enabling bundler cache to speed up the build on travis?

mattbrictson · 2016-12-09T22:18:41Z

@koenpunt Looks great. I don't think we need the bundler cache as part of this PR; we can always add it separately.

Did you want a chance to squash some of the commits? I am fine either way. I'll merge as soon as I get a thumbs-up from you.

koenpunt · 2016-12-10T00:30:52Z

Did you want a chance to squash some of the commits?

No I'm good

mattbrictson · 2016-12-10T03:56:44Z

Merged! Thanks for the awesome PR ✨

koenpunt changed the title ~~Testing multiple version of Rails~~ Testing multiple versions of Rails Jul 5, 2016

This was referenced Dec 9, 2016

Rewrite tests to work on Rails 4.0, 4.1, 4.2, and 5.0 #273

Closed

add test for excluding dashed attributes mbklein/equivalent-xml#36

Open

koenpunt and others added 10 commits December 9, 2016 17:19

setup appraisal to test multiple rails versions

dc0ab26

Test with equivalent-xml and Rails 4.0

45d71c7

I added an `assert_equivalent_xml` ActionView::TestCase, and then I changed all the `assert_equal` to `assert_equivalent_xml`.

disable testing for rails 5 on ruby < 2.2

56c918a

add colored diff for easier debugging

1646788

optimize the diff by sorting the attributes

48a28d6

update locale settings

26d68f2

this to prevent differences in defaults between rails versions

ignore style attribute when comparing nodes

358709e

handle wrapped utf8 hidden field

95a3bc3

as of rails 4.2 the utf8 hidden field is no longer wrapped in a div

ignore data-disable-with attribute

0021e80

for some reason EquivalentXml can't handle attributes with dashes, so a workaround is provided.

handle change type for datetime field

3e6f2be

since rails 5 the type for a datetime field is `datetime-local` instead of `datetime`

mattbrictson suggested changes Dec 9, 2016

View reviewed changes

move require statements to test_helper

2be25fa

koenpunt added 5 commits December 9, 2016 21:44

prevent deprecation warnings

8302467

remove Gemfile.lock

e17fcbd

clear environment check

99f03a8

update README

c5261b2

update CHANGELOG

9fba17d

mattbrictson approved these changes Dec 9, 2016

View reviewed changes

mattbrictson merged commit 5512154 into bootstrap-ruby:master Dec 10, 2016

koenpunt deleted the multi-rails branch January 29, 2017 10:53

mattbrictson mentioned this pull request May 2, 2017

Travis CI builds fail due to Rails 5.1 release #329

Closed

lcreid mentioned this pull request Jan 21, 2018

Support for Rails 5.1 form_with #369

Merged

lcreid mentioned this pull request Mar 12, 2018

Testing form_with and Rails Versions #451

Open

Testing multiple versions of Rails #278

Testing multiple versions of Rails #278

Uh oh!

Conversation

koenpunt commented Jul 5, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

koenpunt commented Jul 5, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mattbrictson commented Jul 6, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mattbrictson commented Jul 7, 2016

Uh oh!

koenpunt commented Jul 7, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mattbrictson commented Jul 7, 2016

Uh oh!

mattbrictson commented Jul 7, 2016

Uh oh!

koenpunt commented Jul 11, 2016

Uh oh!

koenpunt commented Dec 9, 2016

Uh oh!

mattbrictson left a comment

Choose a reason for hiding this comment

Uh oh!

mattbrictson Dec 9, 2016

Choose a reason for hiding this comment

Uh oh!

lcreid Dec 9, 2016

Choose a reason for hiding this comment

Uh oh!

mattbrictson commented Dec 9, 2016

Uh oh!

lcreid commented Dec 9, 2016

Uh oh!

koenpunt commented Dec 9, 2016

Uh oh!

mattbrictson commented Dec 9, 2016

Uh oh!

koenpunt commented Dec 10, 2016

Uh oh!

mattbrictson commented Dec 10, 2016

Uh oh!

Uh oh!

koenpunt commented Jul 5, 2016 •

edited

Loading

koenpunt commented Jul 5, 2016 •

edited

Loading

mattbrictson commented Jul 6, 2016 •

edited

Loading

koenpunt commented Jul 7, 2016 •

edited

Loading