-
Notifications
You must be signed in to change notification settings - Fork 216
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 with_***
helpers
#1687
Add with_***
helpers
#1687
Conversation
c2f1960
to
b01da8d
Compare
lib/rbs/unit_test/with_aliases.rb
Outdated
def with_array(*elements) | ||
return WithEnum.new to_enum(__method__ || raise, *elements) unless block_given? | ||
|
||
yield elements | ||
yield ToArray.new(*elements) | ||
end | ||
|
||
def with_hash(hash = {}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with_array(*elements)
, but not with_hash(**elements)
?
Note: kwargs isn’t Symbols only and that – good?
p('any key' => 'is valid', 420 => 69)
#=> {"any key"=>"is valid", 420=>69}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can pass hash without curly brace with the current definition. So any other good reason to make it kwargs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just prefer consistency.
test/stdlib/test_helper.rb
Outdated
BlankSlate = RBS::UnitTest::Convertibles::BlankSlate | ||
ToIO = RBS::UnitTest::Convertibles::ToIO | ||
ToI = RBS::UnitTest::Convertibles::ToI | ||
ToInt = RBS::UnitTest::Convertibles::ToInt | ||
ToF = RBS::UnitTest::Convertibles::ToF | ||
ToR = RBS::UnitTest::Convertibles::ToR | ||
ToC = RBS::UnitTest::Convertibles::ToC | ||
ToStr = RBS::UnitTest::Convertibles::ToStr | ||
ToS = RBS::UnitTest::Convertibles::ToS | ||
ToSym = RBS::UnitTest::Convertibles::ToSym | ||
ToA = RBS::UnitTest::Convertibles::ToA | ||
ToArray = RBS::UnitTest::Convertibles::ToArray | ||
ToHash = RBS::UnitTest::Convertibles::ToHash | ||
ToPath = RBS::UnitTest::Convertibles::ToPath | ||
CustomRange = RBS::UnitTest::Convertibles::CustomRange | ||
Each = RBS::UnitTest::Convertibles::Each |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
include RBS::UnitTest::Convertibles
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah this is in fact for RBS runtime type checker. I'm not sure if including works for it...
I see potential for property based testing (or fuzzing) in this framework. with_int(count: 100) do |i|
# Yields 100 times random integer value
end |
lib/rbs/unit_test/convertibles.rb
Outdated
module Convertibles | ||
class BlankSlate < BasicObject | ||
instance_methods.each do |im| | ||
next if %i[__send__ __id__].include? im |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might want to remove __id__
from this list, as it doesnt warn when you undef
it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. Will try it.
def each(&block) = @enum.each(&block) | ||
|
||
def and_nil(&block) | ||
self.and(nil, &_ = block) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add return WithEnum.new to_enum(__method__ || raise) unless block
here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and accepts no block call. So it should be ok.
|
||
# An object with `#to_io` method | ||
class ToIO < BlankSlate | ||
@io: untyped |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't these untyped
s be given types? ditto for initialize
and to_xxx
for each of the ToXXX
classes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the type of IO here? I think it's not very clear and decided to keep them untyped.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@io
should be ::IO
; Same for ToStr
and String
, etc
sig/unit_test/with_aliases.rbs
Outdated
|
||
# Yields `::array` objects | ||
# | ||
def with_array: (*untyped elements) { (array[untyped]) -> void } -> void |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should give this a type, like def with_array: [T] (*T elements)
. Same with hash
and range
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Skip range
method because it requires <=>
method.
test/stdlib/File_test.rb
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unintentional inclusion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it's unrelated, but I think it's acceptable to mix it.
b01da8d
to
438e43e
Compare
438e43e
to
980b56d
Compare
This PR moves the helpers like
with_int
fromtest/stdlib
tolib/rbs/unit_test
, so that other gems can use them to test their type definitions.