Add more tests for reopening classes and modules #1314
Conversation
| it "raises TypeError if constant given as class name exists and is not a Module" do | ||
| -> { | ||
| class ClassSpecsNumber | ||
| end | ||
| }.should raise_error(TypeError) | ||
| }.should raise_error(TypeError, /\AClassSpecsNumber is not a class/) | ||
| end | ||
|
|
||
| it "raises TypeError if constant given as class name exists and is not a Class" do | ||
| -> { | ||
| class ClassSpecs::IncludedInObjectWithClass | ||
| end | ||
| }.should raise_error(TypeError, /\AIncludedInObjectWithClass is not a class/) | ||
| end |
There was a problem hiding this comment.
Maybe this should be a single test? Behavior is exactly the same, but there could be semantic differences between a Module constant and a non-Module constant.
There was a problem hiding this comment.
Yeah, as for me it makes sense to have them separate.
trinistr
force-pushed
the
test-class-not-reopening
branch
from
afe799b to
f0b43d2
Compare
language/class_spec.rb
Outdated
| -> { | ||
| class ClassSpecs::IncludedInObjectWithClass | ||
| end | ||
| }.should raise_error(TypeError, /\AIncludedInObjectWithClass is not a class/) |
There was a problem hiding this comment.
suggestion: it makes sense to have a separate fixture module and not to reuse IncludedInObjectWithClass as far as it introduces unnecessary coupling between two tests.
There was a problem hiding this comment.
suggestion: I haven't found any test that would check the same behaviour in general (that's not specific to Object). It makes sense to have such tests for module and class keywords.
There was a problem hiding this comment.
Added tests in non-Objects, though the test descriptions aren't great. I'm actually not sure if there was a difference in old versions between defining a module in top scope and explicitly inside Object (have a feeling there was a difference).
fixtures/class.rb
Outdated
| end | ||
| class Object | ||
| include ClassSpecs::IncludedInObjectWithClass | ||
|
|
There was a problem hiding this comment.
Not sure whether it's a good idea to touch Object - this may affect any other test case. As an option we can extend Object in a subprocess (see usage of ruby_exe helper).
Existing Object reopening comes from 2008 so I wouldn't use it as an example of a good testing practice.
There was a problem hiding this comment.
Should've checked when were the tests written 😅 I've also redone the Module test to reduce pollution.
trinistr
force-pushed
the
test-class-not-reopening
branch
from
f0b43d2 to
0d34ba7
Compare
trinistr
changed the title
trinistr
force-pushed
the
test-class-not-reopening
branch
from
0d34ba7 to
5bc0088
Compare
2 participants
From #1016
3fe6747 added tests for not reopening a Module, but not a Class.
This PR adds two missing tests:
classraising when constant is a Module (similar test exists in module_spec).Update: also tests for reopening modules and classes inside other modules and classes.