Skip to content
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

Nokogiri: Fix XML formatting to make tests green & remove warning #773

Merged
merged 2 commits into from
May 12, 2022

Conversation

carlosantoniodasilva
Copy link

@carlosantoniodasilva carlosantoniodasilva commented May 12, 2022

Fix XML formatting to get Nokogiri tests passing

The mis-formatting was causing this test that was recently added to fail:

1) Error:
Recurly::AddOn::.find#test_0005_must return an addon with percentage-tiered-pricing and usage_timeframe when available:
Recurly::XML::ParseError: Recurly::XML::ParseError
  ./recurly-client-ruby/lib/recurly/xml/nokogiri.rb:7:in `rescue in initialize'
  ./recurly-client-ruby/lib/recurly/xml/nokogiri.rb:4:in `initialize'
  ./recurly-client-ruby/lib/recurly/xml.rb:61:in `initialize'
  ./recurly-client-ruby/lib/recurly/resource/pager.rb:290:in `new'
  ./recurly-client-ruby/lib/recurly/resource/pager.rb:290:in `load_from'
  ./recurly-client-ruby/lib/recurly/resource/pager.rb:171:in `load!'
  ./recurly-client-ruby/lib/recurly/resource/pager.rb:136:in `each'
  ./recurly-client-ruby/spec/recurly/add_on_spec.rb:78:in `first'
  ./recurly-client-ruby/spec/recurly/add_on_spec.rb:78:in `block (3 levels) in <top (required)>'

Fixing the format makes all tests green when running with XML=nokogiri.

Note: the XML file was added recently by #767.

Fix Nokogiri warning by passing the document as argument

This warning is triggered in multiple specs when running with
XML=nokogiri:

./recurly-client-ruby/lib/recurly/xml/nokogiri.rb:11: warning:
Passing a Node as the second parameter to Node.new is deprecated.
Please pass a Document instead, or prefer an alternative constructor
like Node#add_child. This will become an error in a future release
of Nokogiri.

By changing to passing the document instead, as per the warning message,
all tests are green and no warning is issued anymore.

Closes #754.


Test run

rexml

% bundle exec rake
Run options: --seed 13435

# Running:

.......................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................

Finished in 1.462468s, 343.9391 runs/s, 607.1928 assertions/s.

503 runs, 888 assertions, 0 failures, 0 errors, 0 skips

nokogiri

% XML=nokogiri bundle exec rake
Run options: --seed 30196

# Running:

.......................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................

Finished in 0.450451s, 1116.6586 runs/s, 1971.3576 assertions/s.

503 runs, 888 assertions, 0 failures, 0 errors, 0 skips

The mis-formatting was causing this test that was recently added to fail:

    1) Error:
    Recurly::AddOn::.find#test_0005_must return an addon with percentage-tiered-pricing and usage_timeframe when available:
    Recurly::XML::ParseError: Recurly::XML::ParseError
      ./recurly-client-ruby/lib/recurly/xml/nokogiri.rb:7:in `rescue in initialize'
      ./recurly-client-ruby/lib/recurly/xml/nokogiri.rb:4:in `initialize'
      ./recurly-client-ruby/lib/recurly/xml.rb:61:in `initialize'
      ./recurly-client-ruby/lib/recurly/resource/pager.rb:290:in `new'
      ./recurly-client-ruby/lib/recurly/resource/pager.rb:290:in `load_from'
      ./recurly-client-ruby/lib/recurly/resource/pager.rb:171:in `load!'
      ./recurly-client-ruby/lib/recurly/resource/pager.rb:136:in `each'
      ./recurly-client-ruby/spec/recurly/add_on_spec.rb:78:in `first'
      ./recurly-client-ruby/spec/recurly/add_on_spec.rb:78:in `block (3 levels) in <top (required)>'

Fixing the format makes all tests green when running with `XML=nokogiri`.
This warning is triggered in multiple specs when running with
`XML=nokogiri`:

    ./recurly-client-ruby/lib/recurly/xml/nokogiri.rb:11: warning:
    Passing a Node as the second parameter to Node.new is deprecated.
    Please pass a Document instead, or prefer an alternative constructor
    like Node#add_child. This will become an error in a future release
    of Nokogiri.

By changing to passing the document instead, as per the warning message,
all tests are green and no warning is issued anymore.
Copy link
Contributor

@douglasmiller douglasmiller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this, @carlosantoniodasilva

@douglasmiller douglasmiller merged commit 3aa52f9 into recurly:v2 May 12, 2022
@douglasmiller douglasmiller added the V2 V2 Client label Jun 6, 2023
@carlosantoniodasilva carlosantoniodasilva restored the v2 branch July 6, 2023 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
V2 V2 Client
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants