Change `assert_dom` to scope with NodeSet

[seanpdoyle]

Change assert_dom to accept a single Nokogiri::XML::Node or a Nokogiri::XML::NodeSet as an argument without supplementary set of CSS or XPath selectors.

assert_dom document_root_element.at_css("main") do
  assert_dom "h1", "Hello, World"
end

assert_dom css_select("section") do
  assert_dom "h1"
end

This change is in support of making rails/rails#41291 possible:

require "test_helper"
+ require "capybara/minitest"

class ActionDispatch::IntegrationTest
+  include Capybara::Minitest::Assertions
+
+  def page
+    Capybara.string(current_root_element)
+  end
+
+  def within(*arguments, **options, &block)
+    assert_dom page.find(*arguments, **options).native, &block
+  end
end

[seanpdoyle]

I've responded to the latest feedback, thanks for the review.

I think you've picked up on the subtext, but it might be worth stating outright: I've named the method #within to mimic the Capybara::Session method for the sake of symmetry.

That isn't a technical requirement, though. As an alternative, this change could achieve the same outcome by modifying the assert_dom method to accept a single Node argument.

More methods means a larger interface to maintain into the future, and tweaking existing behavior risks breaking backwards compatibility. @rafaelfranca @kaspth Do either of you have a preference on whether or not we should add a new method to the interface?

[seanpdoyle]

@kaspth I've pushed up changes to revert the introduction of #within, and instead change #assert_dom/#assert_select to accept a single node or nodeset argument.

It's currently passing. There is one additional test case I considered adding:

diff --git a/test/selector_assertions_test.rb b/test/selector_assertions_test.rb
index 454a5ab..4f08900 100644
--- a/test/selector_assertions_test.rb
+++ b/test/selector_assertions_test.rb
@@ -227,6 +227,20 @@ class AssertSelectTest < ActiveSupport::TestCase
     assert_dom "aside", text: "other", count: 1
   end

+  def test_assert_dom_with_nodeset_and_options
+    render_html %Q{<div><span>foo</span></div><div><span>bar</span></div>}
+
+    assert_dom css_select("div"), count: 2
+    assert_dom css_select("div"), minimum: 1, maximum: 2
+
+    assert_raises Minitest::Assertion do
+      assert_dom css_select("div"), count: 1
+    end
+    assert_raises Minitest::Assertion do
+      assert_dom css_select("div"), minimum: 3
+    end
+  end
+

Unfortunately, I haven't figured out a way to combine a single node/nodeset argument with the keyword arguments, so that case is consistently failing.

In this case, I'm not sure if it's a use case that'll crop up in the real world, and if callers bumped up against it, they could use the CSS selector substitution the way that it currently works. I think what's most risky is that if we omit support for this style of invocation, the keyword arguments are silently ignored, so the assertions would pass as false positives.

If we think supporting that use case is necessary, I might need some assistance from whoever understands the substitution context / CSS selector code to help carry this work across the finish line.

[seanpdoyle]

I've opened https://github.com/rails/rails/pull/43361 to add a public method to hook Capybara integration with ActionDispatch::IntegationTest onto.

I'd rather explore that option than continue to change rails-dom-testing, so I'll close this PR in its current state.