Wrapped method causing infinite recursion in rcov

I added this to my rails test helper to check for valid HTML
automatically:

Wrap around get to check the markup on :success requests.

alias_method :old_get, :get
def get(*args)
old_get(*args)
if @response.success?
assert_valid_markup
end
end

This works fine with my tests. When running rcov though it creates
infinite recursion in the call to old_get. Has anyone tried anything
of the sort? Is it a bug in rcov?

Pedro.

On 7/27/06, Caleb C. [email protected] wrote:

At a guess, (something is) rcov is also wrapping #get and using the
same name for the old version of the method. At least, that’s always
been the explanation when I’ve encountered these ‘wrapping a method
causes infinite recursion’ problems. Try this way instead, it should
eliminate any possibility that your code is participating in the
recursion.

I tried a different name and it didn’t work so I don’t know what’s
actually happening. But I’d like to know. Anyone?

old_get=instance_method(:get)
define_method(:get) do |*args|
old_get[*args]

I had to replace this line with:

old_get.bind(self).call(*args)

apparently old_get is an UnboundMethod and needs to be bound to an
object before it can work.

if @response.success?
assert_valid_markup
end
end

Thanks for this solution. It’s working again now. This probably means
that there should be an alias_method equivalent to do this. I didn’t
really want to change the method name, I wanted to replace it with
something that could still call the old behavior. 0-step inheritance
so to speak.

This trick was originated by Mauricio F., as far as I know.

Thanks to him too then,

Pedro.

On 7/27/06, Pedro Côrte-Real [email protected] wrote:

infinite recursion in the call to old_get. Has anyone tried anything
of the sort? Is it a bug in rcov?

At a guess, (something is) rcov is also wrapping #get and using the
same name for the old version of the method. At least, that’s always
been the explanation when I’ve encountered these ‘wrapping a method
causes infinite recursion’ problems. Try this way instead, it should
eliminate any possibility that your code is participating in the
recursion.

old_get=instance_method(:get)
define_method(:get) do |*args|
old_get[*args]
if @response.success?
assert_valid_markup
end
end

This trick was originated by Mauricio F., as far as I know.

I don’t like advertising my own work, but…

old_get=instance_method(:get)
define_method(:get) do |*args|
old_get[*args]

This works, unless you want the method to receive a &block.
(Though that’s fixed in Ruby 1.9.)

I would do this:

class Module
def assert_valid_markup(method_name) do
post_condition(method_name) do |obj, method_name, args, block|
# Check it…
end
end
end

class Foo
def get(*args)
# The real stuff…
end

assert_valid_markup :get if $DEBUG
end

I call this assert_valid_markup a “monitor-function” [1]. This
post_condition is defined on the site as well.

gegroet,
Erik V. - http://www.erikveen.dds.nl/

[1] Ruby Monitor-Functions - Or Meta-Meta-Programming in Ruby

On Fri, Jul 28, 2006 at 01:57:56AM +0900, Caleb C. wrote:

This works fine with my tests. When running rcov though it creates
infinite recursion in the call to old_get. Has anyone tried anything
of the sort? Is it a bug in rcov?

At a guess, (something is) rcov is also wrapping #get and using the
same name for the old version of the method. At least, that’s always
been the explanation when I’ve encountered these ‘wrapping a method
causes infinite recursion’ problems. Try this way instead, it should
eliminate any possibility that your code is participating in the
recursion.

rcov doesn’t redefine anything, it just uses a trace_func (or the C
equivalent
when rcovrt is available). In theory, running a program through rcov
shouldn’t
change its behavior, but this is the second time I’m told there’s some
sort of
bothersome interference with Rails (I haven’t yet been able to fix the
first
issue, reported by Assaph M.).

I’d love to solve this, but I first need to reproduce the bug; a naïve
attempt
fails, as expected:

$ rcov --text-counts foo.rb
1

foo.rb

                                                                   | 

0
class Foo; def foo; end end |
5
class Bar < Foo |
2
alias_method :old_foo, :foo |
1
def foo; p 1; old_foo end |
5
end |
0
|
0
Bar.new.foo |
1

If you cannot show me your code, please keep a copy of the codebase
exhibiting
the bug. I might send you a modified rcov for you to test, if I can
figure
this out.

Actually, I think I know what is going on: when running under rcov, the
file
holding your Rails test helper is being require()d twice. If I’m right,
it
should be possible to infer what is going on by tracing #require, with
e.g.

module Kernel
req = instance_method(:require)
define_method(:require) do |*a|
puts “-” * 80
puts "Kernel#require " + a.inspect
puts caller.inspect
req.bind(self).call(*a)
end
end

and then looking for the calls where your test file was loaded. Things
should
be easy until that stage. After that, we’d have to see why rcov
introduces a
difference in the second call.

[Thinking as I write] rcov loads the files supplied in the command line
with
Kernel#load, so if you later #require’ any of them, it would be loaded
twice.
Moreover, Rake’s default test loader also works this way; here’s its
source
code:

ARGV.each { |f| load f unless f =~ /^-/ }

If this hunch is correct, the following patch to bin/rcov could save the
day:

diff -rN -u old-tmp/bin/rcov new-tmp/bin/rcov
— old-tmp/bin/rcov 2006-07-27 21:50:33.000000000 +0200
+++ new-tmp/bin/rcov 2006-07-27 21:50:33.000000000 +0200
@@ -450,7 +450,7 @@
if options.replace_prog_name
$0 = File.basename(File.expand_path(prog))
end

  • load prog
  • require prog
    end

Rake would also have to be changed, but I’d like to make sure the
modification
is appropriate before applying it to rcov and sending jweirich a patch.

old_get=instance_method(:get)
define_method(:get) do |*args|
old_get[*args]
if @response.success?
assert_valid_markup
end
end

This trick was originated by Mauricio F., as far as I know.

It was most probably discovered by some anonymous Japanese Rubyist
before I
popularized it to some extent, though :slight_smile:

On Fri, Jul 28, 2006 at 04:55:15AM +0900, Mauricio F. wrote:

On Fri, Jul 28, 2006 at 01:57:56AM +0900, Caleb C. wrote:

On 7/27/06, Pedro Côrte-Real [email protected] wrote:
rcov doesn’t redefine anything, it just uses a trace_func (or the C
equivalent when rcovrt is available). In theory, running a program through
rcov shouldn’t change its behavior, but this is the second time I’m told
there’s some sort of bothersome interference with Rails (I haven’t yet been
able to fix the first issue, reported by Assaph M.).
========================================
Meanwhile, I’ve figured this one out. Rails was interfering with itself:
rake
test runs unit, functional and integration tests in three separate
processes.
If you load them all simultaneously, you can run into puzzling errors
(reported for the wrong controllers, as
SomeController.new.class != SomeController in some circumstances (!)).

So we’re down to one bug/strange interaction. Let’s address it:

if you are defining the coverage task like this:

Rcov::RcovTask.new(:converage) do |t|
  t.libs << "test"
  t.test_files = FileList['test/**/*_test.rb']
  t.output_dir = 'test/coverage'
  t.verbose = true
  t.rcov_opts << '--rails'
end

the helper could get loaded twice if it is being required in both the
unit
and the functional tests (Rails does ugly things with $: and require).
We
can get some evidence to support that theory by monitoring #require, as
shown
in my previous message.

At any rate, I will soon implement the ability to merge coverage data
from
several runs, allowing you to run the tests separately and obtain a
single
coverage figure.

On 7/27/06, Pedro Côrte-Real [email protected] wrote:

On 7/27/06, Caleb C. [email protected] wrote:

At a guess, (something is) rcov is also wrapping #get and using the
same name for the old version of the method. At least, that’s always
been the explanation when I’ve encountered these ‘wrapping a method
causes infinite recursion’ problems. Try this way instead, it should
eliminate any possibility that your code is participating in the
recursion.

I tried a different name and it didn’t work so I don’t know what’s
actually happening. But I’d like to know. Anyone?

That’s odd, considering that the define_method trick seems to be
working for you. As I said, my diagnosis was just a guess.

old_get=instance_method(:get)
define_method(:get) do |*args|
old_get[*args]

I had to replace this line with:

old_get.bind(self).call(*args)

Ooops! My apologies for posting broken code. Looks like you got the
idea, anyway.

Thanks for this solution. It’s working again now. This probably means
that there should be an alias_method equivalent to do this. I didn’t
really want to change the method name, I wanted to replace it with
something that could still call the old behavior. 0-step inheritance
so to speak.

Wrapping methods seems to be pretty common in ruby. FYI the
define_method trick won’t be able to wrap methods that take a block
until ruby 1.9.

This trick was originated by Mauricio F., as far as I know.

Thanks to him too then,

Just a BTW, he’s the same guy that wrote rcov. So now you have 2
things to thank him for.

On Fri, Jul 28, 2006 at 08:05:37PM +0900, Pedro Côrte-Real wrote:

I just wrapped the call to that assertion so that on every successful
request the HTML is tested. Other than that it’s just normal tests.

Just say so if you still can’t reproduce it.

Could you try to reproduce it with rcov 0.7.0, running the unit,
functional &
integration tests separately as explained in [ruby-talk:206760]? I think
the
file was being require()d with 2 different names, in tests that break
unless
executed in different processes.

On 7/27/06, Mauricio F. [email protected] wrote:

If you cannot show me your code, please keep a copy of the codebase exhibiting
the bug. I might send you a modified rcov for you to test, if I can figure
this out.

I can send you the code if you want but you’ve pretty much seen it
all. assert_valid_markup is a plugin:

http://wiki.rubyonrails.com/rails/pages/Assert+Valid+Markup+Plugin

I just wrapped the call to that assertion so that on every successful
request the HTML is tested. Other than that it’s just normal tests.

Just say so if you still can’t reproduce it.

 req.bind(self).call(*a)

code:
if options.replace_prog_name
$0 = File.basename(File.expand_path(prog))
end

  • load prog
  • require prog
    end

No, doesn’t work :frowning:

It was most probably discovered by some anonymous Japanese Rubyist before I
popularized it to some extent, though :slight_smile:

Thanks anyway.

Pedro.