-
-
Notifications
You must be signed in to change notification settings - Fork 898
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Inheriting from Nokogiri::XML::Node on JRuby (1.6.4/5) fails #560
Comments
Subscribing @taylor and @lgleasain |
Fixed in rev. #5aacda4a5bb758679e07f9f8d4d265202c21c359 As far as I tried, all tests passed after the change. Thanks for reporting. |
Thanks, this test case now passes. Linking the commit for easy reference: https://github.com/tenderlove/nokogiri/commit/5aacda4a5bb758679e07f9f8d4d265202c21c359 |
So, it turns out this isn't the whole story. Attributes are now accessible via #attributes, but not #[], as demonstrated in this extended test case: require 'nokogiri'
MyNode = Class.new Nokogiri::XML::Node
shared_examples_for 'a node' do
let(:node) { node_class.new 'foo', Nokogiri::XML::Document.new }
subject { node }
its(:name) { should == 'foo' }
describe "writing an attribute" do
before { node['foo'] = 'bar' }
describe "accessing via #attributes" do
subject { node.attributes['foo'] }
it { should be_a Nokogiri::XML::Attr }
its(:value) { should == 'bar' }
end
it 'should have the right key' do
node.key?('foo').should be true
end
it 'should be readable via #[]' do
node['foo'].should == 'bar'
end
end
end
describe MyNode do
let(:node_class) { MyNode }
it_should_behave_like 'a node'
end
describe Nokogiri::XML::Node do
let(:node_class) { Nokogiri::XML::Node }
it_should_behave_like 'a node'
end
|
Maybe, "MyNode" is the correct class on line 4 in the test file, right? I changed node_class to MyNode and ran the test. As you said, I got the failures on pure Java but not on libxml version. |
node_class is perfectly correct, and is set toward the bottom of the pasted code. |
Sorry about my ignorance on rspec. I switched back MyNode to node_class. It worked exactly the same as yours. |
Fixed in rev. 875448f The reason of this sort of failures is methods are implemented in subclass of XmlNode. Perhaps, add_namespace, remove_attribute and some other methods have the same problem? |
Why are they implemented in subclasses? That's not the case on the C implementation, as all of these things work. |
Because it is Java. The implementation is following Java's culture. Node is an abstract type. Node might be Attribute, or Text. Does Attribute have attribute? Does Text have attribute? Thus, the attribute related methods used in the given tests should be implemented in Element in Java's culture. If Java's object oriented approach is applied, your tests should inherit Nokogiri::XML::Element. Because inherited class is expected to be an Element. But, Nokogiri is Ruby. Ruby's idea of object oriented is a bit different from Java's. Different behavior from libxml version is considered bug. So, I'll fix those. |
Can you send us tests that use test/unit or minitest? I don't understand this test case. |
Essentially what is required is to run all of the node tests on |
Here it is in test/unit format. All of these fail for me in JRuby. require 'rubygems'
require 'test/unit'
require 'nokogiri'
class TestAdd < Test::Unit::TestCase
MyNode = Class.new Nokogiri::XML::Node
def setup
@node = MyNode.new 'foo', Nokogiri::XML::Document.new
assert @node.name == 'foo'
@node['foo'] = 'bar'
end
def test_a_node_writing_an_attribute_accessing_via_attributes
assert @node.attributes['foo']
end
def test_a_node_writing_an_attribute_accessing_via_key
assert @node.key? 'foo'
end
def test_a_node_writing_an_attribute_accessing_via_brackets
assert @node['foo'] == 'bar'
end
end |
Thanks for the test/unit tests! I'm curious and I'd like to understand our users better: why are you subclassing nokogiri? If you construct an XML document using subclasses then serialize the XML, your information is lost:
The XML representation of both documents is identical, which would indicate that the documents are identical. Yet if we compare the classes of the root nodes, we'll see that the documents are different. Why would this difference be desirable? |
@lgleasain Is that all? Master brach works correctly as expected. |
So this use case is from Blather, where we're doing manual importing of an XML string to create an instance of a particular class from an element name/namespace registration combo. For example, Blather::Stanza::Iq::Command (https://github.com/sprsquish/blather/blob/develop/lib/blather/stanza/iq/command.rb) registers itself for a You can see the importing/registration code here: https://github.com/sprsquish/blather/blob/develop/lib/blather/xmpp_node.rb The Punchblock gem does something similar for the Rayo XMPP extension. @sprsquish can provide a more coherant explanation of this rationale, no doubt. |
@benlangfeld -- everything OK with you on master? |
Not exactly. I get a whole lot of this right now:
The source of this issue is now totally lost on me, but this is suspicious: https://github.com/tenderlove/nokogiri/commit/df274e6b8fb6f929be9313fc4d60174da3b08b67 |
@benlangfeld you are right. That commit surely breaks your test. The reason fo regression is... I didn't add test case for this issue. Sorry. I'll soon add the test provided. |
@benlangfeld I was wrong. That commit doesn't harm node inheritance. I tested two rspec tests provided here. Both successfully finished on master branch. I added the test in rev. be4851d so that regression won't happen. |
Unfortunately I am unable to run the tests fully on JRuby:
|
http://groups.google.com/group/nokogiri-talk/browse_thread/thread/8f58feca3b25fcdc You need to run rake on CRuby first. This generates some necessary files in a source tree. Then, try rake on JRuby. How about your 2 rspec tests pasted here, above? |
So now the tests hang indefinitely:
My rspec tests now pass, but I have a couple of remaining failures in a broader test suite which I will debug and report on within the hour. |
This is the remaining test failure: require 'nokogiri'
MyNode = Class.new Nokogiri::XML::Node
shared_examples_for 'a node' do
let(:node) { node_class.new 'foo', Nokogiri::XML::Document.new }
subject { node }
it { should be_a node_class }
describe 'adding a namespace' do
before { node.add_namespace nil, 'foo:bar' } # A similar failure can be caused on Ruby 1.9.3 here by replacing nil with ''
its(:namespace) { should be_a Nokogiri::XML::Namespace }
it 'should set the right href' do
subject.namespace.href.should == 'foo:bar'
end
end
end
describe MyNode do
let(:node_class) { MyNode }
it_should_behave_like 'a node'
end
describe Nokogiri::XML::Node do
let(:node_class) { Nokogiri::XML::Node }
it_should_behave_like 'a node'
end
|
Ah, 1.9 mode. Same here. Before, rake worked on 1.9 mode though there were many failures. This deserves another ticket. I'll open that. Thanks for trying rake test. |
I fixed the bug in rev. 766db38 . Now, the rspec test posted here on Nov. 18 passed on master. If you have a chance, would you try it? |
I can confirm that this is fixed now. |
The following simple test case:
causes the following failures:
These failures do not occur on CRuby (1.9.2/3) or when using Nokogiri 1.4.7. They have been introduced in Nokogiri 1.5, and unfortunately getting Nokogiri's tests running on JRuby is impossible, as is a bisect, due to the non-linear history between the 1.4 and 1.5 branches.
The text was updated successfully, but these errors were encountered: