heynemann / pyvows

Python implementation of Vows.js
http://pyvows.org
MIT License
133 stars 28 forks source link

Is topic state shared between tests? #18

Open kalbasit opened 12 years ago

kalbasit commented 12 years ago

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 commented 12 years ago

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 commented 12 years ago

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 commented 12 years ago

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...

heynemann commented 12 years ago

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 commented 12 years ago

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 commented 12 years ago

Thanks for the compliments ;)

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

Cheers,

kalbasit commented 12 years ago

Thank you and please do take your time.

Cheers

/cc @duarnad

kalbasit commented 12 years ago

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:

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 commented 11 years ago

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?

jaredly commented 10 years ago

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