[v4] Formatting tests to use heredoc for html blobs

[GBH]

Not attempting to fix anything. Just adjusting formatting so it's easier to work with tests.

• bootstrap_checkbox_test.rb
• bootstrap_fields_test.rb
• bootstrap_form_group_test.rb
• bootstrap_form_test.rb
• bootstrap_other_components_test.rb
• bootstrap_radio_button_test.rb
• bootstrap_selects_test.rb
• special_form_class_models_test.rb

Well, that didn't take forever.

No changes to tests besides heredoc. Added options_range helper method in bootstrap_selects_test because loops are a thing.

Tests are blowing up for Rails that we don't care about anymore. Reason why they started to blow up? assert_equivalent_xml started to see old wrapper around form's hidden field suddenly. I guess adding a whitespace did something. I don't really trust that assertion helper much.

Let's merge this before other PRs come in with tests. It will be a nightmare to resolve conflicts.

Can merge before or after #366. They're don't rely on each other.

[lcreid]

If we merge these changes into master, the form_with PR, and probably others, will be much harder to update and/or merge. Could we agree not to do formatting changes until we've merged or otherwise taken care of the existing PRs? I can try to resubmit PR #352 today or tomorrow if I'm blocking things.

[GBH]

It's easier to adjust outstanding PRs (and they do need adjusting) than to redo heredoc again.

[mattbrictson]

This is a great improvement to the syntax, thanks! I see you removed the <div style="margin:0;padding:0;display:inline"> wrapper from the expectations, which is no longer present in Rails 5+ and therefore is a nice cleanup.

I share the concern that this introduces a lot of conflicts in existing PRs. Let's make sure we close or merge existing v4 PRs before merging this PR. Dealing with the PR backlog should be our first priority.

[GBH]

What PRs will it introduce conflicts in that already don't have conflicts?

[lcreid]

Any PR that changed a test (which should be most of them) will have a conflict. They might not have had one before, since many were submitted against the boostrap-v4 branch.

In my particular case with the form_with helper, there are a tonne of new test cases for form_with that are almost the same as the form_for test cases. With the form_with and form_for test cases formatted the same, it's easier to ensure that the form_with test coverage is equivalent to the form_for test coverage. We'll need that when/if form_for gets removed, and we want to remove the form_for test cases.

That said, the way the test results are formatted now might be easier to work with. I may actually want to format the form_with test cases the way @GBH formatted them. I'm just asking for a day or two to work on the form_with PR and then I'll let you know.

[GBH]

Ok. I just don't want to redo this PR. Sheer volume of html blobs literally took hours to clean up.

[lcreid]

There's good work here and I wouldn't want you to have to redo it. My concern was based on knowing that one PR (#369) had a lot of test changes, but as you've already seen, they didn't have conflicts with your work. I should reformat the tests for #369 to heredocs, but later.

[GBH]

So now there's #370 that will make this PR conflict. You were worrying about old PRs from last year when I was trying to make new PRs better. 🤷‍♂️

[lcreid]

Having refreshed my memory about #369, and having looked at some of the other PRs, I would now support merging this sooner rather than later. The pain of resolving some merge conflicts is mitigated by the fact that the heredoc format is so much easier to work with. Good work, @GBH .

[mattbrictson]

I agree but I would like to get #370 and #344 merged in first. They are from first-time contributors and I'd like to prioritize getting their stuff in without asking them to redo their PRs again. Both are currently free of conflicts.

[mattbrictson]

OK, I am ready for this now that #344 and #370 are merged. Sorry for the conflicts 🙇

[GBH]

It's ready to go. Let's merge this.

[mattbrictson]

Thanks 🙌