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

add spec for TRUE/FALSE/NIL constants that are no longer defined #853

Merged

Conversation

lxxxvi
Copy link
Contributor

@lxxxvi lxxxvi commented Oct 5, 2021

Hello 👋

This PR covers

TRUE/FALSE/NIL constants are no longer defined.

from #823

I am uncertain if I found the right file to put the tests, can you please verify.
I hope I got the version guards right this time.

Again, thanks a lot.

-> { NIL }.should complain(/constant ::NIL is deprecated/)
end
end
end
Copy link
Member

Choose a reason for hiding this comment

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

It should be in language/predefined_spec.rb, could you move them there?
(There were specs for it incorrectly removed in b3ac8c8)


describe "TRUE" do
ruby_version_is "3.0" do
it "raises NameError" do
Copy link
Member

Choose a reason for hiding this comment

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

"is no longer defined" seems clearer

@lxxxvi lxxxvi force-pushed the true-false-nil-constants-are-no-longer-defined branch from 228976e to 1e6d6e9 Compare October 5, 2021 11:38
@@ -1099,6 +1099,8 @@ def obj.foo2; yield; end
DATA IO If the main program file contains the directive __END__, then
the constant DATA will be initialized so that reading from it will
return lines following __END__ from the source file.
FALSE FalseClass Synonym for false (deprecated in Ruby 3).
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
FALSE FalseClass Synonym for false (deprecated in Ruby 3).
FALSE FalseClass Synonym for false (deprecated, removed in Ruby 3).

(same for the others)

end
end

ruby_version_is "2.7"..."3.0" do
Copy link
Member

Choose a reason for hiding this comment

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

Even in 2.5 it was already deprecated, so it should be ruby_version_is ""..."3.0" do

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, thank you!

@lxxxvi
Copy link
Contributor Author

lxxxvi commented Oct 5, 2021

@eregon Thanks for the hint. I moved and adopted the specs accordingly.

Right now it is structured like this:

describe "The predefined global constants" do
  describe "TRUE" do
    ruby_version_is "3.0" {}
    ruby_version_is ""..."3.0" {}
  end

  describe "FALSE" do
    ruby_version_is "3.0" {}
    ruby_version_is ""..."3.0" {}
  end

  describe "NIL" do
    ruby_version_is "3.0" {}
    ruby_version_is ""..."3.0" {}
  end

  # ...
end

Would it make sense to do it that way:

describe "The predefined global constants" do
  ruby_version_is "3.0" do
    describe "TRUE" {}
    describe "FALSE" {}
    describe "NIL" {}
  end

  ruby_version_is ""..."3.0" do
    describe "TRUE" {}
    describe "FALSE" {}
    describe "NIL" {}
  end

  # ...
end

Copy link
Member

@eregon eregon left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

@eregon eregon merged commit ca34b84 into ruby:master Oct 5, 2021
@eregon
Copy link
Member

eregon commented Oct 5, 2021

Re structure I like it as-is for consistency.
Duplication in specs is not necessarily a bad thing, especially trivial things like guards.

@lxxxvi lxxxvi deleted the true-false-nil-constants-are-no-longer-defined branch January 12, 2023 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants