Deep singleton class #1899

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
3 participants
Contributor

LTe commented Sep 12, 2012

I removed check for sc->attached_instance(). We really need this check? Because this check break one spec from singleton_class_spec.rb

Also fixes #1886

@carlosgaldino carlosgaldino and 1 other commented on an outdated diff Sep 12, 2012

spec/ruby/language/singleton_class_spec.rb
@@ -282,6 +282,18 @@ def singleton_method; 1 end
end
end
end
+
+ describe "for a deep singleton class" do
+ before :each do
@carlosgaldino

carlosgaldino Sep 12, 2012

Member

Is this before block really necessary?

Can't we just put it inside the it block? Since it's a single spec I think it's unnecessary to create a before block.

@LTe

LTe Sep 12, 2012

Contributor

Good point, updated

LTe added some commits Sep 12, 2012

@LTe LTe Add spec for deep singleton class e759133
@LTe LTe Attach singleton class
Attach singleton class only when klass() of class is not a Singleton
Class.

Fixes #1886
8f6620a
@LTe LTe Remove passed specs for CI f4c8497
Owner

dbussink commented Sep 12, 2012

Investigated where the original comment came from, it comes from #37. Did you try whether that still works properly with this change?

Contributor

LTe commented Sep 12, 2012

Unfortunately, returns a different value :(

A.cheese: edam
B.cheese: gouda
a.cheese: shropshire blue
b.cheese: cheddar
b_with_meta.cheese: cheshire
b_with_meta.metaclass.cheese: gouda
b_with_meta_meta.cheese: brie
b_with_meta_meta.metaclass.cheese: gouda
b_with_meta_meta.metaclass.metaclass.cheese: wensleydale

but should return

A.cheese: edam
B.cheese: stilton
a.cheese: shropshire blue
b.cheese: cheddar
b_with_meta.cheese: cheshire
b_with_meta.metaclass.cheese: stilton
b_with_meta_meta.cheese: brie
b_with_meta_meta.metaclass.cheese: gouda
b_with_meta_meta.metaclass.metaclass.cheese: wensleydale

Rubinius like gouda :)

Contributor

LTe commented Sep 12, 2012

@dbussink maybe test from #37 should be included to test-suite?

Owner

dbussink commented Sep 12, 2012

Ok, well, we can't go breaking stuff here. It does mean we probably need to have better specs to make sure this breakage would have shown up in the specs.

We probably shouldn't add them as specs 1 on 1 but try to make them as simple as possible

Contributor

LTe commented Sep 12, 2012

Ok, I will close this pull request for now and prepare better specs.

LTe closed this Sep 12, 2012

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