Replace Nokogiri with REXML in the JUnit formatter #1621
Conversation
In #1454, we welcomed a newly-revamped JUnit formatter which has a dependency on Nokogiri. Unfortunately, this had led us to problems getting InSpec included in Chef omnibus builds (see chef/chef#5937) because Chef is using Ruby 2.4.1 and the Nokogiri maintainers have not yet released a windows binary gem that supports Ruby 2.4.x. This has led to breaking builds in Chef's CI platform and would block the acceptance of chef/chef#5937. This change replaces Nokogiri use with REXML instead. While REXML can be slower than Nokogiri, it does not require native extensions and is supported on all Chef platforms. Signed-off-by: Adam Leff <adam@leff.co>
There was a problem hiding this comment.
Thank you @adamleff for this improvement. I've added some comments. Can REXML be used in conjunction with nokogiri?
| @@ -39,7 +39,6 @@ Gem::Specification.new do |spec| | |||
| spec.add_dependency 'mixlib-log' | |||
| spec.add_dependency 'sslshake', '~> 1' | |||
| spec.add_dependency 'parallel', '~> 1.9' | |||
| spec.add_dependency 'nokogiri', '~> 1.6' | |||
There was a problem hiding this comment.
do we need REXML as dependency then?
There was a problem hiding this comment.
No, REXML is in the Ruby stdlib.
| doc = Nokogiri::XML(out.stdout) | ||
| doc.errors.length.must_equal 0 | ||
| doc = REXML::Document.new(out.stdout) | ||
| doc.has_elements?.must_equal true |
There was a problem hiding this comment.
Isn't that a different test? before we checked the errors in doc but now we just check that doc has elements?
There was a problem hiding this comment.
No. Nokogiri will happily try and parse anything you throw at it and return a Nokogiri::XML::Document object. It will not raise an exception unless you enable strict parsing.
So the way to determine if Nokogiri parsed the XML successfully is to call #errors on it, or set string parsing and catch the exception.
REXML is somewhat similar... it will take any string or IO object and try to return a REXML::Document object. You can tell if it's valid XML if it has any REXML::Element objects contained within it.
| doc = Nokogiri::XML(out.stdout) | ||
| doc.errors.length.must_equal 0 | ||
| doc = REXML::Document.new(out.stdout) | ||
| doc.has_elements?.must_equal true |
There was a problem hiding this comment.
No. Nokogiri will happily try and parse anything you throw at it and return a Nokogiri::XML::Document object. It will not raise an exception unless you enable strict parsing.
So the way to determine if Nokogiri parsed the XML successfully is to call #errors on it, or set string parsing and catch the exception.
REXML is somewhat similar... it will take any string or IO object and try to return a REXML::Document object. You can tell if it's valid XML if it has any REXML::Element objects contained within it.
|
@chris-rock there is nothing blocking someone from using Nokogiri and REXML in the same library/application. REXML is in the stdlib and all its classes are namespaced accordingly. |
In #1454, we welcomed a newly-revamped JUnit formatter which has
a dependency on Nokogiri. Unfortunately, this had led us to problems
getting InSpec included in Chef omnibus builds (see chef/chef#5937)
because Chef is using Ruby 2.4.1 and the Nokogiri maintainers have
not yet released a windows binary gem that supports Ruby 2.4.x.
This has led to breaking builds in Chef's CI platform and would
block the acceptance of chef/chef#5937.
This change replaces Nokogiri use with REXML instead. While REXML
can be slower than Nokogiri, it does not require native extensions
and is supported on all Chef platforms.
/cc @jkerry