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

2.4 ruby is not working with structured_warnings #62

Open
wants to merge 9 commits into
from

Conversation

Projects
None yet
3 participants
@razboev

razboev commented Feb 23, 2017

see schmidt/structured_warnings#7 for more info
So, this library was removed

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Feb 23, 2017

Coverage Status

Coverage decreased (-2.0%) to 96.535% when pulling e841889 on razboev:master into db48c35 on evolve75:master.

coveralls commented Feb 23, 2017

Coverage Status

Coverage decreased (-2.0%) to 96.535% when pulling e841889 on razboev:master into db48c35 on evolve75:master.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Feb 24, 2017

Coverage Status

Coverage decreased (-1.7%) to 96.782% when pulling 362a28b on razboev:master into db48c35 on evolve75:master.

coveralls commented Feb 24, 2017

Coverage Status

Coverage decreased (-1.7%) to 96.782% when pulling 362a28b on razboev:master into db48c35 on evolve75:master.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Feb 24, 2017

Coverage Status

Coverage decreased (-1.7%) to 96.782% when pulling b8e9d77 on razboev:master into db48c35 on evolve75:master.

coveralls commented Feb 24, 2017

Coverage Status

Coverage decreased (-1.7%) to 96.782% when pulling b8e9d77 on razboev:master into db48c35 on evolve75:master.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Feb 24, 2017

Coverage Status

Coverage decreased (-1.7%) to 96.782% when pulling fe8be23 on razboev:master into db48c35 on evolve75:master.

coveralls commented Feb 24, 2017

Coverage Status

Coverage decreased (-1.7%) to 96.782% when pulling fe8be23 on razboev:master into db48c35 on evolve75:master.

Show outdated Hide outdated test/test_binarytree.rb
assert_not_empty(capture_output{@root.leftChild = @left_child2}.last)
assert_not_empty(capture_output{@root.rightChild = @right_child2}.last)
#ruby self include private policy changed in 2.3
if RUBY_VERSION<'2.3.0'

This comment has been minimized.

@abotalov

abotalov Feb 24, 2017

IMO this test shows the presence of an issue in this library in Ruby >= 2.3.
The bug is that to_snake_case method that was intended to be private is indeed public because of a behavior change in Ruby.
So a proper fix would arguably be to make to_snake_case method private. It seems method_missing and to_snake_case can be just made not nested methods (i.e. def self.included can be removed).

By the way, I filed an issue for Ruby here - https://bugs.ruby-lang.org/issues/13249

@abotalov

abotalov Feb 24, 2017

IMO this test shows the presence of an issue in this library in Ruby >= 2.3.
The bug is that to_snake_case method that was intended to be private is indeed public because of a behavior change in Ruby.
So a proper fix would arguably be to make to_snake_case method private. It seems method_missing and to_snake_case can be just made not nested methods (i.e. def self.included can be removed).

By the way, I filed an issue for Ruby here - https://bugs.ruby-lang.org/issues/13249

Show outdated Hide outdated test/test_tree.rb
@@ -310,8 +310,7 @@ def test_length_is_size
# Test the <=> operator.
def test_spaceship
require 'structured_warnings'
StandardWarning.disable # Disable the warnings for using integers as node names

This comment has been minimized.

@abotalov

abotalov Feb 24, 2017

Empty lines at the start and the end of the method are arguably not needed.

@abotalov

abotalov Feb 24, 2017

Empty lines at the start and the end of the method are arguably not needed.

Show outdated Hide outdated test/test_tree.rb
@@ -956,8 +954,7 @@ def test_content
# instead of depth. This method has been deprecated in this release and may be removed in the future.
def test_depth
begin
require 'structured_warnings'
assert_warn(DeprecatedMethodWarning) { do_deprecated_depth }
assert_not_empty(capture_output { do_deprecated_depth }.last)
rescue LoadError

This comment has been minimized.

@abotalov

abotalov Feb 24, 2017

rescuing LoadError doesn't seem to be needed because require 'structured_warnings' was removed.

@abotalov

abotalov Feb 24, 2017

rescuing LoadError doesn't seem to be needed because require 'structured_warnings' was removed.

require 'structured_warnings'
DeprecatedMethodWarning.disable do
assert(@root.isRoot?) # Test if the original method is really called

This comment has been minimized.

@abotalov

abotalov Feb 24, 2017

Previous version of a unit test checked that original method is really called when a camel-cased method is run. So a new version should arguably continue to check it.

@abotalov

abotalov Feb 24, 2017

Previous version of a unit test checked that original method is really called when a camel-cased method is run. So a new version should arguably continue to check it.

begin
require 'stringio'
$stdout = StringIO.new
assert_warn(DeprecatedMethodWarning) { @root.send('printTree') }

This comment has been minimized.

@abotalov

abotalov Feb 24, 2017

Previous version of the unit test checked that printTree method is available. New version doesn't check it anymore. It should arguably check it.

@abotalov

abotalov Feb 24, 2017

Previous version of the unit test checked that printTree method is available. New version doesn't check it anymore. It should arguably check it.

@abotalov

This comment has been minimized.

Show comment
Hide comment
@abotalov

abotalov Feb 24, 2017

Some additional comments:

  • README.md should arguably be updated to remove information about Ruby 1.8.x support and structured_warnings

abotalov commented Feb 24, 2017

Some additional comments:

  • README.md should arguably be updated to remove information about Ruby 1.8.x support and structured_warnings
@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Feb 27, 2017

Coverage Status

Coverage decreased (-0.01%) to 98.511% when pulling 06f53ee on razboev:master into db48c35 on evolve75:master.

coveralls commented Feb 27, 2017

Coverage Status

Coverage decreased (-0.01%) to 98.511% when pulling 06f53ee on razboev:master into db48c35 on evolve75:master.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Dec 21, 2017

Coverage Status

Coverage decreased (-0.01%) to 98.511% when pulling 5e678b3 on razboev:master into db48c35 on evolve75:master.

coveralls commented Dec 21, 2017

Coverage Status

Coverage decreased (-0.01%) to 98.511% when pulling 5e678b3 on razboev:master into db48c35 on evolve75:master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment