Forum: RSpec Testing discovered a problem in my code

Announcement (2017-05-07): www.ruby-forum.com is now read-only since I unfortunately do not have the time to support and maintain the forum any more. Please see rubyonrails.org/community and ruby-lang.org/en/community for other Rails- und Ruby-related community platforms.
059ed46172a087063ce26250e44c8627?d=identicon&s=25 Fernando Perez (fernando)
on 2009-04-16 13:51
When trying to test using sqlite in-memory in ran into a problem:

- rake test raises an error on a test
- running the failing test alone works perfectly.

So what's the problem? here is the method giving the trouble:

def self.expiry_date_for(user)
  @expiry_date_cache ||= find_if_expiry_date_for(user)
end

That cached method is also called by another file, and therefore sets
the @expiry_date_cache to some value therefore not acting correctly.

So is my code flawed or is it the testing framework that doesn't clear
correctly the cached variable? In production, would my code work
correctly?
Ef58e2f39136bbf134bcf352447c2a58?d=identicon&s=25 Matthew Krom (Guest)
on 2009-04-16 14:15
(Received via mailing list)
Your single test may be relying on database data that is set up (and
left
there) by a different test.  (In your non-sqlite database, the data may
be
sitting there as the result of previous testing activity, so the single
test
may pass there and not on sqlite)
059ed46172a087063ce26250e44c8627?d=identicon&s=25 Fernando Perez (fernando)
on 2009-04-16 14:20
Matthew Krom wrote:
> Your single test may be relying on database data that is set up (and
The tests don't hit the database. Only one previous test hits the same
method and forces the class to set this instance variable.

But I thing that I really have a flaw in my code, as this class instance
variable will be set once by the process and will keep the same value as
long as the process is running. I should probably rewrite it to an
instance variable.
059ed46172a087063ce26250e44c8627?d=identicon&s=25 Fernando Perez (fernando)
on 2009-04-16 14:46
> But I thing that I really have a flaw in my code
I confirm, my code had a big flaw.
Ef58e2f39136bbf134bcf352447c2a58?d=identicon&s=25 Matthew Krom (Guest)
on 2009-04-16 14:57
(Received via mailing list)
Also, just noticed your class-caching isn't keyed off user.  (I'm also
honestly not sure what @ instead of @@ means inside a self. class
method;
I'd have to look that up and write specs to test it!)

Something like this may work better.

@expiry_date_cache ||= {}
@expiry_date_cache[user.id] ||= find_if_expiry_date_for(user)

(Also Rails 2.something has a memoize feature which does key off
arguments--see the Railscast.  Not sure if it works for class methods).

Matt
059ed46172a087063ce26250e44c8627?d=identicon&s=25 Fernando Perez (fernando)
on 2009-04-16 15:00
> @expiry_date_cache ||= {}
> @expiry_date_cache[user.id] ||= find_if_expiry_date_for(user)

From the beginning my code was silly. The cache has to be tied to an
object that only exists for the current request being processed. So I
have refactored it.

tdd/bdd/rspec/test::unit/whatever helped me to discover this.
Ef58e2f39136bbf134bcf352447c2a58?d=identicon&s=25 Matthew Krom (Guest)
on 2009-04-16 15:00
(Received via mailing list)
I missed that; sorry.  When code uses class-cached data for performance,
I've developed a testing pattern that explicitly clears class-cached
data as
part of the test data setup.  It does require careful attention.  I'd be
interested in what others do!
39100495c9937c39b2e0c704444e1b4a?d=identicon&s=25 Pat Maddox (Guest)
on 2009-04-16 16:49
(Received via mailing list)
You're setting an instance variable on a class, which is a global
variable and is not garbage collected. The state you set is maintained
across runs, screwing things up.

If you set config.cache_classes to false in environments/test.rb I
think it ought to reload the classes each time.

There's no way for rspec to know that you would want to clear that
variable, you have to do that yourself. This is one example of how
global variables suck.

Also your code doesn't make sense to me. I'd you called it twice, each
with different users, you would get the same result which is prob not
what you want.

Pat
059ed46172a087063ce26250e44c8627?d=identicon&s=25 Fernando Perez (fernando)
on 2009-04-16 16:52
> This is one example of how global variables suck.
>
> Also your code doesn't make sense to me. I'd you called it twice, each
> with different users, you would get the same result which is prob not
> what you want.

Yup, that's why I corrected it. Now the method is an instance method of
User, so I call it as following:

current_user.membership_expiry_date
48641c4be1fbe167929fb16c9fd94990?d=identicon&s=25 Mark Wilden (Guest)
on 2009-04-16 17:32
(Received via mailing list)
On Thu, Apr 16, 2009 at 5:46 AM, Matthew Krom <matthew.krom@gmail.com>
wrote:

> Also, just noticed your class-caching isn't keyed off user.  (I'm also
> honestly not sure what @ instead of @@ means inside a self. class method;
> I'd have to look that up and write specs to test it!)

@var inside a class method is just an instance variable like any
other. There's one for each object instantiated from its class and no
other object can access it. The difference is that in this case, the
object is a class - i.e., it is instantiated from the Class class.
It's call a class instance variable.

@@var is a class variable and can be accessed from both the class
itself and objects instantiated from it.

///ark
48641c4be1fbe167929fb16c9fd94990?d=identicon&s=25 Mark Wilden (Guest)
on 2009-04-16 18:26
(Received via mailing list)
On Thu, Apr 16, 2009 at 7:33 AM, Pat Maddox <pat.maddox@gmail.com>
wrote:

> You're setting an instance variable on a class, which is a global
> variable

I wouldn't call a class instance variable global. It's accessible to
only one object, after all.

> and is not garbage collected. The state you set is maintained
> across runs, screwing things up.

But that's a good point.

///ark
39100495c9937c39b2e0c704444e1b4a?d=identicon&s=25 Pat Maddox (Guest)
on 2009-04-16 19:29
(Received via mailing list)
On Thursday, April 16, 2009, Mark Wilden <mark@mwilden.com> wrote:
> On Thu, Apr 16, 2009 at 7:33 AM, Pat Maddox <pat.maddox@gmail.com> wrote:
>
>> You're setting an instance variable on a class, which is a global
>> variable
>
> I wouldn't call a class instance variable global. It's accessible to
> only one object, after all.

Neither would I :)  A class, being referenced by a constant, is a
global variable.

Pat
48641c4be1fbe167929fb16c9fd94990?d=identicon&s=25 Mark Wilden (Guest)
on 2009-04-16 20:17
(Received via mailing list)
On Thu, Apr 16, 2009 at 10:23 AM, Pat Maddox <pat.maddox@gmail.com>
wrote:
> global variable.
Sorry I misunderstood you - the perils of relative pronouns. :)

///ark
This topic is locked and can not be replied to.