Support class redefinitions in runtime for rbs test mode about rbs HOT 7 CLOSED

@HoneyryderChuck
Thank you for the explanation. I understand the issue now.

When remove_const/const_set is executed and the class object is swapped, the old class remains in RBS::Test::TypeCheck#const_cache. Consequently, when calling #is_a? for the Types::ClassInstance check, the test fails because the object_id differs.

If we can hook into remove_const to clear RBS::Test::TypeCheck#const_cache, it seems we can solve the problem. This RBS::Test::TypeCheck#const_cache exists as a memoized cache within the RBS::Test::Tester::MethodCallTester instance, so we need to find this instance. However, currently, there doesn't seem to be a way to retrieve it.

For a makeshift solution, you could do something like the following, which should work for now:

diff --git a/lib/rbs/test/observer.rb b/lib/rbs/test/observer.rb
index ff023871..05cc0394 100644
--- a/lib/rbs/test/observer.rb
+++ b/lib/rbs/test/observer.rb
@@ -13,6 +13,10 @@ module RBS
         def register(key, object = nil, &block)
           @@observers[key] = object || block
         end
+
+        def observers
+          @@observers
+        end
       end
     end
   end
diff --git a/lib/rbs/test/setup.rb b/lib/rbs/test/setup.rb
index 1a8041c9..16c30e47 100644
--- a/lib/rbs/test/setup.rb
+++ b/lib/rbs/test/setup.rb
@@ -75,6 +75,21 @@ TracePoint.trace :end do |tp|
   end
 end

+hook = Module.new{
+  def remove_const(name)
+    super
+  end
+}
+Module.prepend(hook)
+
+TracePoint.new(:call) do |tp|
+  name = tp.binding.local_variable_get(:name)
+  type_name = TypeName("::#{tp.self}::#{name}")
+  RBS::Test::Observer.observers.values.each do |method_call_tester|
+    method_call_tester.check.const_cache.delete(type_name)
+  end
+end.enable(target: hook.instance_method(:remove_const))
+
 at_exit do
   if $!.nil? || $!.is_a?(SystemExit) && $!.success?
     if tester.targets.empty?

Alternatively, a very simple fix would be to just stop caching.

diff --git a/lib/rbs/test/type_check.rb b/lib/rbs/test/type_check.rb
index ac0edbc3..6ed8d134 100644
--- a/lib/rbs/test/type_check.rb
+++ b/lib/rbs/test/type_check.rb
@@ -208,11 +208,9 @@ module RBS
       end

       def get_class(type_name)
-        const_cache[type_name] ||= begin
-                                     Object.const_get(type_name.to_s)
-                                   rescue NameError
-                                     nil
-                                   end
+        Object.const_get(type_name.to_s)
+      rescue NameError
+        nil
       end

       def is_double?(value)

@soutaro How about this?

from rbs.

@HoneyryderChuck Thank you for reporting the problem.

Hmm, rbs/test/setup is loaded before other libraries, net/http is defined, and webmock assigns new class to Net::HTTP...
It seems like we need to add a hook to setup runtime type checkers after loading webmock.

I also wonder if @ksss has any idea about this?

from rbs.

It is possible to hook remove_const and const_set with TracePoint, but I'm not yet clear on the potential issues.
Do you have a way to reproduce the problem?

from rbs.

I couldn't find narrow down the smallest possible reproduction, unfortunately. The error can be reproduced with httpx integration, test suite, as httpx performs the same "remove_const/const_set" trick in its webmock integrations. Just: clone the repo, bundle install, set up the rbs env vars and run bundle exec rake integration_tests.

from rbs.

Disable the caching! Seems like there were no strong reason to introduce it... #356

from rbs.

I'll send PR soon.

from rbs.

Thx everyone

from rbs.