sparklemotion/nokogiri

(upstream libxml2) test errors

Closed this issue · 6 comments

flavorjones commented

Seeing these errors testing against GNOME/libxml2@6a49bb7

  1) Failure:
Nokogiri::XML::CDATA::.new#test_0003_has nil content when passed nil [test/xml/test_cdata.rb:35]:
Expected "" to be nil.

  2) Failure:
ad hoc node reparenting behavior::#add_next_sibling::with a text node after#test_0001_should not defensively dup the 'after' text node [test/xml/test_node_reparenting.rb:665]:
Expected: "after"
  Actual: "xafter"

  3) Failure:
ad hoc node reparenting behavior::#add_previous_sibling::with a text node before#test_0001_should not defensively dup the 'before' text node [test/xml/test_node_reparenting.rb:626]:
Expected: "before"
  Actual: "beforex"

Also I'm seeing a memory leak (as reported by ruby_memcheck):

120 bytes in 1 blocks are definitely lost in loss record 20,941 of 29,501
  malloc (at /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
  objspace_xmalloc0 (gc.c:12337)
  xmlNewElem (tree.c:2069)
  xmlNewDocNodeEatName (tree.c:2174)
  xmlSAX2StartElementNs (SAX2.c:2081)
  xmlParseStartTag2.constprop.0 (parser.c:9490)
  xmlParseElementStart (parser.c:9886)
  xmlParseContentInternal (parser.c:9728)
  xmlParseInNodeContext (parser.c:12446)
 *in_context (xml_node.c:2183)
flavorjones commented

The CDATA test started failing with GNOME/libxml2@9991fae

The node reparenting tests started failing with GNOME/libxml2@4ccd3eb

flavorjones commented

The memory leak started getting reported with GNOME/libxml2@7e46242

flavorjones commented

OK, here's what the fixes look like:

  • CData.new no longer accepts nil as the content argument
  • update node GC lifecycle to avoid leaking fragment nodes
  • update reparenting tests to accept the coalesced text nodes

PR imminent.

nwellnhof commented

I will readdress the changes to xmlAddChild. If it was used with linked nodes before, it would most likely lead to a corrupted tree, but now I realize that it did work in some scenarios like moving a list node by node.

  • ❤️1
nwellnhof commented

xmlAddChild was changed here to fix the issue you were seeing: https://gitlab.gnome.org/GNOME/libxml2/-/commit/ea0ee365465d0d5fd4d70ebbe7ff7b4471088c7b

I also reverted some changes to xmlAddPrevSibling and xmlAddNextSibling here: https://gitlab.gnome.org/GNOME/libxml2/-/commit/f43197fca7e0f6fc2c63d7fd1dd2a4230540bfc6

  • ❤️1