rspec users - Apr 2009 - Testing discovered a problem in my code

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?
-- 
Posted via http://www.ruby-forum.com/.

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)

On Thu, Apr 16, 2009 at 7:51 AM, Fernando Perez <lists at
ruby-forum.com>wrote:
> 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?
> --
> Posted via http://www.ruby-forum.com/.
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://rubyforge.org/pipermail/rspec-users/attachments/20090416/5962a675/attachment.html>

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.
-- 
Posted via http://www.ruby-forum.com/.

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!

On Thu, Apr 16, 2009 at 8:20 AM, Fernando Perez <lists at
ruby-forum.com>wrote:
> 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.
> --
> Posted via http://www.ruby-forum.com/.
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://rubyforge.org/pipermail/rspec-users/attachments/20090416/e495a24d/attachment.html>

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

On Thu, Apr 16, 2009 at 7:51 AM, Fernando Perez <lists at
ruby-forum.com>wrote:
> 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?
> --
> Posted via http://www.ruby-forum.com/.
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
>


-- 
matthew.krom at gmail.com
617 852 5130
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://rubyforge.org/pipermail/rspec-users/attachments/20090416/57650836/attachment.html>

> But I thing that I really have a flaw in my code
I confirm, my code had a big flaw.
-- 
Posted via http://www.ruby-forum.com/.

> @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.
-- 
Posted via http://www.ruby-forum.com/.

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

On Thursday, April 16, 2009, Fernando Perez <lists at ruby-forum.com>
wrote:
> 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?
> --
> Posted via http://www.ruby-forum.com/.
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
>

On Thu, Apr 16, 2009 at 5:46 AM, Matthew Krom <matthew.krom at 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

> 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
-- 
Posted via http://www.ruby-forum.com/.

On Thu, Apr 16, 2009 at 7:33 AM, Pat Maddox <pat.maddox at 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

On Thursday, April 16, 2009, Mark Wilden <mark at mwilden.com>
wrote:
> On Thu, Apr 16, 2009 at 7:33 AM, Pat Maddox <pat.maddox at 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

On Thu, Apr 16, 2009 at 10:23 AM, Pat Maddox <pat.maddox at gmail.com>
wrote:
> On Thursday, April 16, 2009, Mark Wilden <mark at mwilden.com> wrote:
>> On Thu, Apr 16, 2009 at 7:33 AM, Pat Maddox <pat.maddox at
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.
Sorry I misunderstood you - the perils of relative pronouns. :)

///ark