`ActionDispatch::Testing::TestResponse#parsed_body` parse HTML with Nokogiri #47144

seanpdoyle · 2023-01-25T21:27:33Z

Motivation / Background

Prior to this commit, the only out-of-the-box parsing that ActionDispatch::Testing::TestResponse#parsed_body supported was for application/json requests. This meant that response.body == response.parsed_body for HTML requests.

Using parsed_body for JSON requests supports Hash#fetch, Hash#dig, and Ruby 3.2 destructuring assignment and pattern matching.

Detail

The introduction of Nokogiri support for pattern
matching poses an opportunity to make assertions about the structure of the HTML response.

On top of that, there is ongoing work to introduce pattern matching support in MiniTest.

get "/posts"
response.content_type         # => "text/html; charset=utf-8"
response.parsed_body.class    # => Nokogiri::HTML5::Document
response.parsed_body.to_html  # => "<!DOCTYPE html>\n<html>\n..."

Checklist

Before submitting the PR make sure the following are checked:

This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
Tests are added or updated if you fix a bug or add a feature.
CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

actionpack/lib/action_dispatch/testing/request_encoder.rb

rafaelfranca · 2023-01-25T21:37:14Z

actionpack/CHANGELOG.md

+    get "/posts"
+    response.content_type         # => "text/html; charset=utf-8"
+    response.parsed_body.class    # => Nokogiri::HTML5::Document
+    response.parsed_body.to_html  # => "<!DOCTYPE html>\n<html>\n..."


Does this mean that people that were expecting strings will have to rewrite their tests?

Yes, it would be a breaking change.

people that were expecting strings will have to rewrite their tests?

I wasn't sure about this. Since response.parsed_body and response.body return the same type (String), would they be motivated to use response.parsed_body?

Would upgrading from 7.0 to 7.1 be an appropriate time to introduce a breaking change? Would a breaking change require introducing a configuration? Something like action_dispatch.html_parsed_body_as_document = true that <= 7.0 applications would set to false?

I think it is fine to just explicitly document that the return type of that method is going to change.

seanpdoyle · 2023-01-25T22:02:44Z

actionpack/actionpack.gemspec

@@ -35,6 +35,7 @@ Gem::Specification.new do |s|

  s.add_dependency "activesupport", version

+  s.add_dependency "nokogiri", ">= 1.8.5"


This version was copied from actiontext/actiontext.gemspec.

Gemfile.lock

flavorjones

As noted in my inline comment, let's please not assume Nokogiri::HTML5 exists, and check for it so we can fall back to Nokogiri::HTML for platforms and versions that don't support it.

flavorjones · 2023-01-26T19:00:25Z

actionpack/lib/action_dispatch/testing/request_encoder.rb

@@ -50,6 +52,7 @@ def self.register_encoder(mime_name, param_encoder: nil, response_parser: nil)
      @encoders[mime_name] = new(mime_name, param_encoder, response_parser)
    end

+    register_encoder :html, response_parser: -> body { Nokogiri::HTML5.parse(body) }


Just a note that the Nokogiri::HTML5 module/namespace will not exist for Nokogiri < 1.12.0 or for any JRuby user (see sparklemotion/nokogiri#2227).

I think we should take care to gracefully fall back to Nokogiri::HTML.parse(body) unless defined?(::Nokogiri::HTML5)

actionpack/lib/action_dispatch/testing/request_encoder.rb

…okogiri Prior to this commit, the only out-of-the-box parsing that `ActionDispatch::Testing::TestResponse#parsed_body` supported was for `application/json` requests. This meant that `response.body == response.parsed_body` for HTML requests. ```ruby get "/posts" response.content_type # => "text/html; charset=utf-8" response.parsed_body.class # => Nokogiri::HTML5::Document response.parsed_body.to_html # => "<!DOCTYPE html>\n<html>\n..." ``` Using `parsed_body` for JSON requests supports `Hash#fetch`, `Hash#dig`, and Ruby 3.2 destructuring assignment and pattern matching. The introduction of [Nokogiri support for pattern matching][nokogiri-pattern-matching] poses an opportunity to make assertions about the structure of the HTML response. On top of that, there is ongoing work to [introduce pattern matching support in MiniTest][minitest-pattern-matching]. [nokogiri-pattern-matching]: sparklemotion/nokogiri#2523 [minitest-pattern-matching]: minitest/minitest#936

flavorjones

LGTM, thanks for making those changes!

Use the helpers introduced in rails-dom-testing 2.2.0 instead of managing the HTML parsers as was done in rails#48523. See also related rails#47144 / ad79ed0

Add notes for [rails#47144][] to the 7.1 Release Notes. Additionally, in the time since that was merged, both Nokogiri and Minitest have merged the PRs mentioned to integrate support for Ruby's Pattern matching (sparklemotion/nokogiri#2523 and minitest/minitest#936, respectively). This commit adds coverage for those new assertions, and incorporates guidance into the release notes. [rails#47144]: rails#47144

Use the helpers introduced in rails-dom-testing 2.2.0 instead of managing the HTML parsers as was done in rails#48523. See also related rails#47144 / ad79ed0

Add notes for [rails#47144][] to the 7.1 Release Notes. Additionally, in the time since that was merged, both Nokogiri and Minitest have merged the PRs mentioned to integrate support for Ruby's Pattern matching (sparklemotion/nokogiri#2523 and minitest/minitest#936, respectively). This commit adds coverage for those new assertions, and incorporates guidance into the release notes. [rails#47144]: rails#47144

As of [rails#47144][], `response.parsed_body` supports parsing both `format: :json` and `format: :html` responses. This commit updates the documentation to reflect that. Additionally, it cribs the `ActionDispatch::IntegrationTest` documentation that mentions `register_encoder`, and copies it to the method definition so that the documentation entry for that method has descriptive information. [rails#47144]: rails#47144

rails-bot bot added the actionpack label Jan 25, 2023

rafaelfranca reviewed Jan 25, 2023

View reviewed changes

seanpdoyle force-pushed the action-dispatch-testing-parsed-body-html branch 2 times, most recently from 250c6d4 to 3bc7d1d Compare January 25, 2023 21:52

seanpdoyle commented Jan 25, 2023

View reviewed changes

Gemfile.lock Outdated Show resolved Hide resolved

rafaelfranca approved these changes Jan 25, 2023

View reviewed changes

Gemfile.lock Outdated Show resolved Hide resolved

flavorjones suggested changes Jan 26, 2023

View reviewed changes

seanpdoyle force-pushed the action-dispatch-testing-parsed-body-html branch from 3bc7d1d to 8b3835e Compare January 27, 2023 00:31

seanpdoyle commented Jan 27, 2023

View reviewed changes

actionpack/lib/action_dispatch/testing/request_encoder.rb Show resolved Hide resolved

seanpdoyle force-pushed the action-dispatch-testing-parsed-body-html branch from 8b3835e to 180fe3e Compare January 27, 2023 14:31

seanpdoyle force-pushed the action-dispatch-testing-parsed-body-html branch from 180fe3e to ad79ed0 Compare January 28, 2023 04:52

flavorjones approved these changes Jan 30, 2023

View reviewed changes

rafaelfranca merged commit c819075 into rails:main Jan 30, 2023

seanpdoyle deleted the action-dispatch-testing-parsed-body-html branch January 30, 2023 22:54

zzak mentioned this pull request Feb 2, 2023

Add script to generate This Week In Rails rails/website#98

Merged

seanpdoyle mentioned this pull request Aug 22, 2023

Document response.parsed_body in 7.1 release notes #49001

Merged

4 tasks

seanpdoyle mentioned this pull request Oct 6, 2023

Document ActionDispatch::IntegrationTest#register_encoder [ci skip] #49513

Open

4 tasks

r7kamura mentioned this pull request Nov 10, 2023

Support Nokogiri::HTML.parse and Nokogiri::HTML5.parse on Rails/ResponseParsedBody rubocop/rubocop-rails#1181

Merged

9 tasks

seanpdoyle mentioned this pull request Dec 18, 2023

Treat as: :html tests request params as :url_encoded_form #50390

Open

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`ActionDispatch::Testing::TestResponse#parsed_body` parse HTML with Nokogiri #47144

`ActionDispatch::Testing::TestResponse#parsed_body` parse HTML with Nokogiri #47144

seanpdoyle commented Jan 25, 2023 •

edited

rafaelfranca Jan 25, 2023

seanpdoyle Jan 25, 2023

rafaelfranca Jan 25, 2023

seanpdoyle Jan 25, 2023

flavorjones left a comment •

edited

flavorjones Jan 26, 2023

flavorjones left a comment

		@@ -35,6 +35,7 @@ Gem::Specification.new do \|s\|

		s.add_dependency "activesupport", version

		s.add_dependency "nokogiri", ">= 1.8.5"

ActionDispatch::Testing::TestResponse#parsed_body parse HTML with Nokogiri #47144

ActionDispatch::Testing::TestResponse#parsed_body parse HTML with Nokogiri #47144

Conversation

seanpdoyle commented Jan 25, 2023 • edited

Motivation / Background

Detail

Checklist

rafaelfranca Jan 25, 2023

Choose a reason for hiding this comment

seanpdoyle Jan 25, 2023

Choose a reason for hiding this comment

rafaelfranca Jan 25, 2023

Choose a reason for hiding this comment

seanpdoyle Jan 25, 2023

Choose a reason for hiding this comment

flavorjones left a comment • edited

Choose a reason for hiding this comment

flavorjones Jan 26, 2023

Choose a reason for hiding this comment

flavorjones left a comment

Choose a reason for hiding this comment

`ActionDispatch::Testing::TestResponse#parsed_body` parse HTML with Nokogiri #47144

`ActionDispatch::Testing::TestResponse#parsed_body` parse HTML with Nokogiri #47144

seanpdoyle commented Jan 25, 2023 •

edited

flavorjones left a comment •

edited