From 3b3fba61478c419d8e27d468688d1d1eb0e3cbba Mon Sep 17 00:00:00 2001 From: cfis Date: Sun, 5 Apr 2026 18:29:50 -0700 Subject: [PATCH] Support different xml parsing backends - including nokogiri and libxml-ruby --- lib/ruby_memcheck.rb | 1 + lib/ruby_memcheck/test_task_reporter.rb | 11 +--- lib/ruby_memcheck/xml_backend.rb | 55 +++++++++++++++++++ .../xml_backend/libxml_backend.rb | 36 ++++++++++++ .../xml_backend/nokogiri_backend.rb | 39 +++++++++++++ .../ruby_memcheck_suppression_test.rb | 43 ++++++++------- test/ruby_memcheck/valgrind_error_test.rb | 41 +++++++------- test/ruby_memcheck/xml_backend_test.rb | 50 +++++++++++++++++ test/test_helper.rb | 22 ++++++++ 9 files changed, 249 insertions(+), 49 deletions(-) create mode 100644 lib/ruby_memcheck/xml_backend.rb create mode 100644 lib/ruby_memcheck/xml_backend/libxml_backend.rb create mode 100644 lib/ruby_memcheck/xml_backend/nokogiri_backend.rb create mode 100644 test/ruby_memcheck/xml_backend_test.rb diff --git a/lib/ruby_memcheck.rb b/lib/ruby_memcheck.rb index c476b7c..9a892c0 100644 --- a/lib/ruby_memcheck.rb +++ b/lib/ruby_memcheck.rb @@ -14,6 +14,7 @@ require "ruby_memcheck/valgrind_error" require "ruby_memcheck/suppression" require "ruby_memcheck/version" +require "ruby_memcheck/xml_backend" module RubyMemcheck class << self diff --git a/lib/ruby_memcheck/test_task_reporter.rb b/lib/ruby_memcheck/test_task_reporter.rb index bde2321..adb5c94 100644 --- a/lib/ruby_memcheck/test_task_reporter.rb +++ b/lib/ruby_memcheck/test_task_reporter.rb @@ -63,18 +63,11 @@ def valgrind_xml_files end def parse_valgrind_output - require "nokogiri" - @errors = [] + backend = XmlBackend.create valgrind_xml_files.each do |file| - reader = Nokogiri::XML::Reader(File.open(file)) do |config| # rubocop:disable Style/SymbolProc - config.huge - end - reader.each do |node| - next unless node.name == "error" && node.node_type == Nokogiri::XML::Reader::TYPE_ELEMENT - - error_xml = Nokogiri::XML::Document.parse(node.outer_xml).root + backend.each_error(file) do |error_xml| error = ValgrindError.new(configuration, loaded_binaries, error_xml) next if error.skip? diff --git a/lib/ruby_memcheck/xml_backend.rb b/lib/ruby_memcheck/xml_backend.rb new file mode 100644 index 0000000..2348184 --- /dev/null +++ b/lib/ruby_memcheck/xml_backend.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +module RubyMemcheck + module XmlBackend + LOAD_ERROR_MSG = + "ruby_memcheck requires either nokogiri or libxml-ruby to parse Valgrind XML output" + + def self.create + backend_class.new + end + + def self.backend_class + @backend_class ||= detect_backend_class + end + + class NodeAdapter + def initialize(node) + @node = node + end + + def at_xpath(path) + wrap(find_one(path)) + end + + def xpath(path) + find_all(path).map { |node| wrap(node) } + end + + def content + @node.content + end + + def name + @node.name + end + + private + + def wrap(node) + node && self.class.new(node) + end + end + + def self.detect_backend_class + require "ruby_memcheck/xml_backend/libxml_backend" + LibxmlBackend + rescue LoadError + require "ruby_memcheck/xml_backend/nokogiri_backend" + NokogiriBackend + rescue LoadError + raise LoadError, LOAD_ERROR_MSG + end + private_class_method :detect_backend_class + end +end diff --git a/lib/ruby_memcheck/xml_backend/libxml_backend.rb b/lib/ruby_memcheck/xml_backend/libxml_backend.rb new file mode 100644 index 0000000..94a6b09 --- /dev/null +++ b/lib/ruby_memcheck/xml_backend/libxml_backend.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require "libxml-ruby" + +module RubyMemcheck + module XmlBackend + class LibxmlNodeAdapter < NodeAdapter + private + + def find_one(path) + @node.find(path).first + end + + def find_all(path) + @node.find(path) + end + end + + class LibxmlBackend + def each_error(file) + reader = LibXML::XML::Reader.file(file) + + while reader.read + next unless reader.node_type == LibXML::XML::Reader::TYPE_ELEMENT + next unless reader.name == "error" + + yield parse(reader.read_outer_xml) + end + end + + def parse(xml) + LibxmlNodeAdapter.new(LibXML::XML::Document.string(xml).root) + end + end + end +end diff --git a/lib/ruby_memcheck/xml_backend/nokogiri_backend.rb b/lib/ruby_memcheck/xml_backend/nokogiri_backend.rb new file mode 100644 index 0000000..4af7a45 --- /dev/null +++ b/lib/ruby_memcheck/xml_backend/nokogiri_backend.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +require "nokogiri" + +module RubyMemcheck + module XmlBackend + class NokogiriNodeAdapter < NodeAdapter + private + + def find_one(path) + @node.at_xpath(path) + end + + def find_all(path) + @node.xpath(path) + end + end + + class NokogiriBackend + def each_error(file) + File.open(file) do |io| + reader = Nokogiri::XML::Reader(io) do |config| # rubocop:disable Style/SymbolProc + config.huge + end + + reader.each do |node| + next unless node.name == "error" && node.node_type == Nokogiri::XML::Reader::TYPE_ELEMENT + + yield parse(node.outer_xml) + end + end + end + + def parse(xml) + NokogiriNodeAdapter.new(Nokogiri::XML::Document.parse(xml).root) + end + end + end +end diff --git a/test/ruby_memcheck/ruby_memcheck_suppression_test.rb b/test/ruby_memcheck/ruby_memcheck_suppression_test.rb index d458873..f346eb5 100644 --- a/test/ruby_memcheck/ruby_memcheck_suppression_test.rb +++ b/test/ruby_memcheck/ruby_memcheck_suppression_test.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require "test_helper" -require "nokogiri" module RubyMemcheck class RubyMemcheckSuppressionTest < Minitest::Test @@ -10,21 +9,6 @@ def setup end def test_given_a_suppression_node - suppression = ::Nokogiri::XML(<<~EOF).at_xpath("//suppression") - - - insert_a_suppression_name_here - Memcheck:Leak - match-leak-kinds: definite - malloc - objspace_xmalloc0 - ruby_xmalloc0 - /usr/lib/libX11.so.6.3.0 - ruby_xmalloc_body - ruby_xmalloc - - - EOF expected = <<~EOF { insert_a_suppression_name_here @@ -37,10 +21,29 @@ def test_given_a_suppression_node fun:ruby_xmalloc } EOF - assert_equal( - expected, - RubyMemcheck::Suppression.new(@configuration, suppression).to_s, - ) + + RubyMemcheck.test_xml_backends.each do |backend| + suppression = backend.parse(<<~EOF).at_xpath("//suppression") + + + insert_a_suppression_name_here + Memcheck:Leak + match-leak-kinds: definite + malloc + objspace_xmalloc0 + ruby_xmalloc0 + /usr/lib/libX11.so.6.3.0 + ruby_xmalloc_body + ruby_xmalloc + + + EOF + + assert_equal( + expected, + RubyMemcheck::Suppression.new(@configuration, suppression).to_s, + ) + end end end end diff --git a/test/ruby_memcheck/valgrind_error_test.rb b/test/ruby_memcheck/valgrind_error_test.rb index 2f4cff5..b4a6f11 100644 --- a/test/ruby_memcheck/valgrind_error_test.rb +++ b/test/ruby_memcheck/valgrind_error_test.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require "test_helper" -require "nokogiri" module RubyMemcheck class ValgrindErrorTest < Minitest::Test @@ -10,29 +9,31 @@ def setup end def test_raises_when_suppressions_generated_but_not_configured - output = ::Nokogiri::XML(<<~XML).at_xpath("//error") - - 0x1ab8 - 1 - Leak_DefinitelyLost - - 48 bytes in 1 blocks are definitely lost in loss record 6,841 of 11,850 - 48 - 1 - + RubyMemcheck.test_xml_backends.each do |backend| + output = backend.parse(<<~XML) + + 0x1ab8 + 1 + Leak_DefinitelyLost + + 48 bytes in 1 blocks are definitely lost in loss record 6,841 of 11,850 + 48 + 1 + - - + + - - - - XML + + + + XML - error = assert_raises do - RubyMemcheck::ValgrindError.new(@configuration, [], output) + error = assert_raises do + RubyMemcheck::ValgrindError.new(@configuration, [], output) + end + assert_equal(ValgrindError::SUPPRESSION_NOT_CONFIGURED_ERROR_MSG, error.message) end - assert_equal(ValgrindError::SUPPRESSION_NOT_CONFIGURED_ERROR_MSG, error.message) end end end diff --git a/test/ruby_memcheck/xml_backend_test.rb b/test/ruby_memcheck/xml_backend_test.rb new file mode 100644 index 0000000..407e03e --- /dev/null +++ b/test/ruby_memcheck/xml_backend_test.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +require "test_helper" + +module RubyMemcheck + class XmlBackendTest < Minitest::Test + XML = <<~XML + + + Leak_DefinitelyLost + + 100 bytes in 1 blocks are definitely lost + + + + c_test_one_memory_leak + /tmp/ruby_memcheck_c_test_one.so + + + + + XML + + def test_prefers_libxml_when_available + assert_equal("RubyMemcheck::XmlBackend::LibxmlBackend", XmlBackend.backend_class.name) + end + + def test_each_error_with_each_available_backend + Tempfile.create(["ruby_memcheck", ".xml"]) do |file| + file.write(XML) + file.flush + + RubyMemcheck.test_xml_backends.each do |backend| + errors = [] + + backend.each_error(file.path) do |error| + errors << error + end + + assert_equal(1, errors.length) + assert_equal("Leak_DefinitelyLost", errors.first.at_xpath("kind").content) + assert_equal( + "100 bytes in 1 blocks are definitely lost", + errors.first.at_xpath("xwhat/text").content, + ) + end + end + end + end +end diff --git a/test/test_helper.rb b/test/test_helper.rb index 0172c8c..307e9f1 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -5,6 +5,28 @@ require "minitest/autorun" +module RubyMemcheck + def self.test_xml_backends + backends = [] + + begin + require "ruby_memcheck/xml_backend/nokogiri_backend" + backends << XmlBackend::NokogiriBackend.new + rescue LoadError + nil + end + + begin + require "ruby_memcheck/xml_backend/libxml_backend" + backends << XmlBackend::LibxmlBackend.new + rescue LoadError + nil + end + + backends + end +end + if ENV["CI"] require "etc" ENV["NCPU"] ||= Etc.nprocessors.to_s