Module#class_variables should accept an inherit flag. Tests from MRI …

…assume an order from class_variables (@eregon said he will patch upstream so we don't lose this change on next sync)
jruby · Jun 2, 2015 · 95a9d88 · 95a9d88
1 parent 15b545e
commit 95a9d88
Showing 3 changed files with 22 additions and 11 deletions.
diff --git a/core/src/main/java/org/jruby/RubyModule.java b/core/src/main/java/org/jruby/RubyModule.java
@@ -2944,29 +2944,41 @@ public IRubyObject remove_class_variable19(ThreadContext context, IRubyObject na
         return remove_class_variable(context, name);
     }
 
-    /** rb_mod_class_variables
-     *
-     */
+    @Deprecated
+    public RubyArray class_variables19(ThreadContext context) {
+        return class_variables(context);
+    }
+
+    @JRubyMethod(name = "class_variables")
     public RubyArray class_variables(ThreadContext context) {
-        return class_variables19(context);
+        Ruby runtime = context.runtime;
+        RubyArray ary = runtime.newArray();
+
+        Collection<String> names = classVariablesCommon(true);
+        for (String name : names) {
+            ary.add(runtime.newSymbol(name));
+        }
+        return ary;
     }
 
     @JRubyMethod(name = "class_variables")
-    public RubyArray class_variables19(ThreadContext context) {
+    public RubyArray class_variables(ThreadContext context, IRubyObject inherit) {
         Ruby runtime = context.runtime;
         RubyArray ary = runtime.newArray();
 
-        Collection<String> names = classVariablesCommon();
+        Collection<String> names = classVariablesCommon(inherit.isTrue());
         for (String name : names) {
             ary.add(runtime.newSymbol(name));
         }
         return ary;
     }
 
-    private Collection<String> classVariablesCommon() {
+
+    private Collection<String> classVariablesCommon(boolean inherit) {
         Set<String> names = new HashSet<String>();
         for (RubyModule p = this; p != null; p = p.getSuperClass()) {
             names.addAll(p.getClassVariableNameList());
+            if (!inherit) break;
         }
         return names;
     }

diff --git a/test/mri/excludes/TestModule.rb b/test/mri/excludes/TestModule.rb
@@ -1,7 +1,6 @@
 exclude :test_alias, "needs investigation"
 exclude :test_attr, "needs investigation"
 exclude :test_attr_inherited_visibility, "IR regression"
-exclude :test_class_variables, "needs investigation"
 exclude :test_classpath, "needs investigation"
 exclude :test_const_get_inherited, "needs investigation"
 exclude :test_const_set_invalid_name, "UTF-16 invalid name is accepted as ok"
@@ -15,4 +14,4 @@
 exclude :test_private_constant_reopen, "needs investigation"
 exclude :test_private_constant_with_no_args, "needs investigation"
 exclude :test_protected_singleton_method, "needs investigation"
-exclude :test_undef, "needs investigation"
+exclude :test_undef, "needs investigation"
diff --git a/test/mri/ruby/test_module.rb b/test/mri/ruby/test_module.rb
@@ -1729,8 +1729,8 @@ def test_class_variables
     m2.send(:include, m)
     m2.class_variable_set(:@@bar, 2)
     assert_equal([:@@foo], m.class_variables)
-    assert_equal([:@@bar, :@@foo], m2.class_variables)
-    assert_equal([:@@bar, :@@foo], m2.class_variables(true))
+    assert_equal([:@@bar, :@@foo], m2.class_variables.sort)
+    assert_equal([:@@bar, :@@foo], m2.class_variables(true).sort)
     assert_equal([:@@bar], m2.class_variables(false))
   end