Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is topic state shared between tests? #18

Open
kalbasit opened this issue Jan 31, 2012 · 10 comments
Open

Is topic state shared between tests? #18

kalbasit opened this issue Jan 31, 2012 · 10 comments

Comments

@kalbasit
Copy link

Hello,

I'm new to pyVows (and to Python for that matter). I'm coming from Ruby but need to develop a module for work. That said, I'm a bit confused to find out that the state of the tested object (the topic) is being shared between tests.

Look at the code and the test output to know exactly what I mean:

from pyvows import Vows, expect

class Fixnum(object):
    def __init__(self):
        self.num = 0

    def add(self, num):
        self.num += num

@Vows.batch
class FixnumTest(Vows.Context):
    def topic(self):
        return Fixnum()

    class Init(Vows.Context):
        def should_be_numeric(self, topic):
            expect(topic.num).to_be_numeric()

        def should_init_to_zero(self, topic):
            expect(topic.num).to_equal(0)

        def should_be_able_to_change_number(self, topic):
            topic.num = 9
            expect(topic.num).to_equal(9)

    class Add(Vows.Context):
        def should_respond_to_add(self, topic):
            expect(topic.add).to_be_a_function()

        def should_be_able_to_add_a_number(self, topic):
            topic.num = 1
            topic.add(2)
            expect(topic.num).to_equal(3)
$ pyvows /tmp/fixnum.py                                                                                                                                                                                                                                    
 ============
 Vows Results
 ============


    Fixnum test
      Init
      ✗ should init to zero
          Expected topic(9) to equal 0.

          Traceback (most recent call last):
            File "/Users/wael/.pythonbrew/pythons/Python-2.7.2/lib/python2.7/site-packages/pyvows/runner.py", line 150, in async_run_vow
              result = member(context_instance, topic)
            File "/Users/wael/.pythonbrew/pythons/Python-2.7.2/lib/python2.7/site-packages/pyvows/runner.py", line 240, in wrapper
              ret = method(*args, **kw)
            File "/tmp/fixnum.py", line 20, in should_init_to_zero
              expect(topic.num).to_equal(0)
            File "/Users/wael/.pythonbrew/pythons/Python-2.7.2/lib/python2.7/site-packages/pyvows/core.py", line 52, in assert_topic
              return getattr(Vows.Assert, method_name)(self.topic, *args, **kw)
            File "/Users/wael/.pythonbrew/pythons/Python-2.7.2/lib/python2.7/site-packages/pyvows/core.py", line 171, in exec_assertion
              raise VowsAssertionError(raw_msg, *args)
          VowsAssertionError: Expected topic(9) to equal 0.

          found in /tmp/fixnum.py at line 19

  ✗ OK » 4 honored • 1 broken (0.003353s)

Am i doing something wrong ?

@fabiomcosta
Copy link
Collaborator

You should not change your topic. The topic is executed once, and the same object returned from the topic function is used to run all the defined specs in parallel.

If you need this kind of thing, you need to make it sure that your specs are running sequentially, like this:

@Vows.batch
class FixnumTest(Vows.Context):
    def topic(self):
        return Fixnum()

    class Init:
        def should_be_numeric(self, topic):
            expect(topic.num).to_be_numeric()

        def should_init_to_zero(self, topic):
            expect(topic.num).to_equal(0)

        class ChangingValue:
            def topic(self, old_topic):
                old_topic.num = 9
                return old_topic

            def should_be_able_to_change_number(self, topic):
                expect(topic.num).to_equal(9)

...

Each "class level" will execute sequentially and your tests will work.

Give us your feedback when you solve your problem.

@heynemann
Copy link
Owner

That's because in order to use pyvows (as is true for vows.js, or even rspec
subject mode
) you need to use subject-oriented testing (or BDD in a
different way).

 RSPEC
  describe CheckingAccount, "with $50" do

     
     
subject { CheckingAccount.new(:amount => 50, :currency => :USD) }
it { should have_a_balance_of(50, :USD) }
it { should_not be_overdrawn }

 end

In the above test, the CheckingAccount instance is also shared among the two
tests.

That's perfectly valid, because this way you only execute your code (which
probably is the slowest part of running your tests) once, and not twice.

In pyVows if you do not define a topic for a given context, it will inherit
the topic from the previous context, thus giving the behavior you described.

Hope this all clarifies the situation.

Cheers,
Bernardo Heynemann

@kalbasit
Copy link
Author

I think this is very wrong. This creates a level of dependency between the tests, especially that they are running in parallel and can be very destructive.

In my opinion every test should get a pristine copy of what the topic returns. The same goes for nested contexts (they should receive a pristine copy of what the parent topic returns.

Look at the demonstration in rspec below. This is how tests should behave. Each apply the logic onto a pristine copy of the subject:

class MyFixnum
  attr_accessor :num

  def initialize
    @num = 0
  end

  def add(num)
    @num += num
  end
end

describe MyFixnum do
  subject { MyFixnum.new }

  it "should be 0 even outside the context" do
    subject.num.should == 0
  end

  context 'init' do
    it "should be 0" do
      subject.num.should == 0
    end
  end

  context 'setting' do
    it "should allow me to set the num" do
      subject.num = 9
      subject.num.should == 9
    end

    it "should again be 0" do
      subject.num.should == 0
    end
  end

  context 'add' do
    it { should respond_to :add }

    it "should add to num" do
      subject.num = 1
      subject.add 2
      subject.num.should == 3
    end

    it "should also be 0, adding to it should not change the pristine copy" do
      subject.num.should == 0
    end
  end

  it "should be 0 even outside the context and even at the end of all tests" do
    subject.num.should == 0
  end
end

and the output:

$ rspec -cf doc /tmp/my_fixnum.rb

MyFixnum
  should be 0 even outside the context
  should be 0 even outside the context and even at the end of all tests
  init
    should be 0
  setting
    should allow me to set the num
    should again be 0
  add
    should respond to #add
    should add to num
    should also be 0, adding to it should not change the pristine copy

Finished in 0.0016 seconds
8 examples, 0 failures

UPDATE: Added more tests to push the tests even further...

kalbasit added a commit to kalbasit/pyvows that referenced this issue Jan 31, 2012
kalbasit added a commit to kalbasit/pyvows that referenced this issue Feb 1, 2012
This has broken 26 tests which is expected because tests were written to
make the topic a single object shared between all tests, the team has to
make a decision about which way should the tests be ran, sharing the
same topic or with a pristine topic ?

refs heynemann#18
@heynemann
Copy link
Owner

I get what you are saying @eMxyzptlk. The issue here is the way Python deals with objects.

In order to provide pyvows with pristine copies of the topic, we'd have to use deepcopy which is not reliable. (It’s also really slow…and one of the things we love about pyvows is that it’s FAST.)

We used to do it with deepcopy, but we had very hard to debug issues with it. I'm not familiar with how this is done in Ruby (how RSpec manages to create a copy of the object).

I'll give deepcopy another shot. It's been a while, and pyvows has changed considerably since we last tried. Maybe we can get the best of both worlds.

Thanks again for your feedback.

Bernardo Heynemann

@kalbasit
Copy link
Author

kalbasit commented Feb 2, 2012

Hey @heynemann,

Thank you for your reply, the commit b0be6d8 above uses deepcopy to close the 3d23d10 failing test. However, this commit broke 26 tests (because they validate/uses the fact that the object does change between tests).

My colleague (@duarnad) actually suggested something we could probably use to make those 26 tests pass again. We could make the pristine topic an optional feature, triggered by probably a tag like @pristineTopic before a batch or a context.

No problem, I fell in love with PyVows on first sight (really close to RSpec and I can't live without RSpec :)) very nice project and it deserves all the time and attention we can afford, Keep up the good Work!!

Wael

@heynemann
Copy link
Owner

Thanks for the compliments ;)

Implementing a decorator might be a good idea... Let me just think about it a little more, ok?

Cheers,

@kalbasit
Copy link
Author

kalbasit commented Feb 2, 2012

Thank you and please do take your time.

Cheers

/cc @duarnad

@kalbasit
Copy link
Author

kalbasit commented Feb 2, 2012

I think I might know how RSpec does that, but I do not know if that is even possible in Python.

What it does is:

  • create a new instance of Object called instance
  • re-declare every declared variable (using let(), and from inside before(:each) and before(:all) and the subject) as instance variables of the new instance (instance.instance_variable_set(name, value))
  • then executes the block given to it() using instance_eval() on the instance instance.instance_eval(&block).

I'm not sure if that's exactly how they do it, but it is one way because what's in the instance can't affect the ones outside the instance. You can read the code here https://github.com/rspec/rspec-core/blob/master/lib/rspec/core/example_group.rb#L235-403

Anyway best of luck @heynemann If you need anything please let me know.

Wael

@heynemann
Copy link
Owner

I'd really like to provide this capability, but still haven't had any success in finding a way of cloning the object between calls. Any ideas?

@ghost ghost assigned heynemann Jan 31, 2013
@jaredly
Copy link

jaredly commented Apr 3, 2014

can't you just run topic() again? That's the way beforeEach works in mocha.js, for example...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants