stalniy / bdd-lazy-var

Provides UI for testing frameworks such as mocha, jasmine and jest which allows to define lazy variables and subjects.
MIT License
162 stars 14 forks source link

Lazy variables initialized in unexpected order when `beforeEach` is used #14

Closed dkreft closed 8 years ago

dkreft commented 8 years ago

I'm not sure how to properly describe this, but it looks like beforeEach is causing lazily-created objects to be built too soon and variables in nested contexts are ignored. Here's a test that I wrote to demonstrate the problem:

'use strict'

import { expect } from 'chai'

class UserProvidedValue {
  constructor(value) {
    this.value = value
  }

  doStuff() {
  }
}

describe('UserProvidedValue', () => {
  def('value', () => null)
  def('model', () => {
    console.log(`Building new model with ${ $value }`)
    return new UserProvidedValue($value)
  })

  describe('.doStuff()', () => {
    def('value', () => {
      console.log('making wrong value')
      return 'WRONG VALUE'
    })

    beforeEach(() => {
      // Commenting out this code will result in a successful run
      console.log('calling .doStuff()')
      $model.doStuff()
    })

    describe('the value', () => {
      subject(() => {
        console.log('calling $model.value')
        return $model.value
      })

      context('when the value is supplied', () => {
        def('value', () => {
          console.log('making right value')
          return 'right value'
        })

        it('has the right value', () => {
          expect($subject).to.equal($value)
        })
      })
    })
  })
})

Here's the output from this test:

  UserProvidedValue
    .doStuff()
      the value
        when the value is supplied
calling .doStuff()
making wrong value
Building new model with WRONG VALUE
calling $model.value
making right value
          1) has the right value

  0 passing (11ms)
  1 failing

  1) UserProvidedValue .doStuff() the value when the value is supplied has the right value:

      AssertionError: expected 'WRONG VALUE' to equal 'right value'
      + expected - actual

      -WRONG VALUE
      +right value

      at Context.<anonymous> (lazy-var-test.js:45:31)

[09:47:50] 'test' errored after 185 ms
[09:47:50] Error in plugin 'gulp-mocha'
Message:
    1 test failed.

Compare this to an analogous spec written in rspec:

class UserProvidedValue
  def initialize(value)
    @value = value
  end

  def value
    @value
  end

  def do_stuff
  end
end

describe "UserProvidedValue" do
  let(:value) { nil }
  let(:model) {
    puts "Building new model with #{ value }"
    UserProvidedValue.new(value)
  }

  describe '#do_stuff' do
    let(:value) {
      puts 'making wrong value'
      'WRONG VALUE'
    }

    before :each do
      puts "calling #do_stuff"
      model.do_stuff
    end

    describe 'the value' do
      subject {
        puts "calling model.value"
        model.value
      }

      context 'when the value is supplied' do
        let(:value) {
          puts 'making right value'
          'right value'
        }

        it 'has the right value' do
          expect(subject).to eql(value)
        end
      end
    end
  end
end

The rspec test behaves as expected:

 rspec -f d ~/Desktop/rspec-test.rb

UserProvidedValue
  #do_stuff
    the value
      when the value is supplied
calling #do_stuff
making right value
Building new model with right value
calling model.value
        has the right value

Finished in 0.00134 seconds (files took 0.12593 seconds to load)
1 example, 0 failures
stalniy commented 8 years ago

@dkreft thanks for the issue.

I'll check it in a while but I remember that this part was done intentionally. Read the spec: https://github.com/stalniy/bdd-lazy-var/blob/master/spec/interface_examples.js#L112

I'll figure out why I did this and will provide an option or fix for this

dkreft commented 8 years ago

Thanks for looking into this. If this is intentional, it's unfortunate for those of us who are used to the way rspec works. :-(

-dan

On Tue, Jul 12, 2016 at 9:50 PM, Sergii Stotskyi notifications@github.com wrote:

@dkreft https://github.com/dkreft thanks for the issue.

I'll check it in a while but I remember that this part was done intentionally. Read the spec: https://github.com/stalniy/bdd-lazy-var/blob/master/spec/interface_examples.js#L112

I'll figure out why I did this and will provide an option or fix for this

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/stalniy/bdd-lazy-var/issues/14#issuecomment-232255862, or mute the thread https://github.com/notifications/unsubscribe/AAQpaTEgagcZI7G6QyFEf_G2AjKM_I3lks5qVG6qgaJpZM4JJkLd .[image: Web Bug from https://github.com/notifications/beacon/AAQpadX-hnQEbzAKAqIWRV9p2DGp3eBwks5qVG6qgaJpZM4JJkLd.gif]

stalniy commented 8 years ago

@dkreft I found a use-case:

describe('FTP page', function() {
  subject(function() {
     return Page.open('ftp');
  });

  describe('FTP groups', function() {
    beforeEach(function() {
      subject().tabs.switchTo('ftp-groups');
    });

    describe('when saved', function() {
      // some tests
    });

    describe('Import', function() {
      // other tests
    });

    describe('Export', function() {
      // few more tests
    });

    describe('FTP groups list', function() {
      subject(function() {
        return subject().ftpGroups();
      });

      // tests for FTP groups
    });
  });
});

So, in these tests I want to go to FTP page and then for FTP groups tests I need to switch tab to "FTP Groups" that's why I added beforeEach inside FTP groups describe. But for FTP groups list section I want to redefine subject.

Using rspec approach for vars initialization beforeEach for FTP groups list section would fail because ftpGroups list doesn't have tabs.

How would you write the same scenario in rspec style?

P.S.: If we can cover this with rspec scenario, then I will update code to follow that. If that's not possible without breaking changes (what is more likely) I will add additional option to def function.

By default it will work as it is and if you want it to behave like rspec:

def.on = 'currentSpec'
dkreft commented 8 years ago

Hrmmm....I'll have to think about this, but right now I'm a bit tied up by other issues.

-dan

On Wed, Jul 13, 2016 at 2:42 PM, Sergii Stotskyi notifications@github.com wrote:

@dkreft https://github.com/dkreft I found a use-case:

describe('FTP page', function() { subject(function() { return Page.open('ftp'); });

describe('FTP groups', function() { beforeEach(function() { subject().tabs.switchTo('ftp-groups'); });

describe('when saved', function() {
  // some tests
});

describe('Import', function() {
  // other tests
});

describe('Export', function() {
  // few more tests
});

describe('FTP groups list', function() {
  subject(function() {
    return subject().ftpGroups();
  });

  // tests for FTP groups
});

}); });

So, in these tests I want to go to FTP page and then for FTP groups tests I need to switch tab to "FTP Groups" that's why I added beforeEach inside FTP groups describe. But for FTP groups list section I want to redefine subject.

Using rspec approach for vars initialization beforeEach for FTP groups list section would fail because ftpGroups list doesn't have tabs.

How would you write the same scenario in rspec style?

P.S.: If we can cover this with rspec scenario, then I will update code to follow that. If that's not possible without breaking changes (what is more likely) I will add additional option to def function.

By default it will work as it is and if you want it to behave like rspec:

def.on = 'currentSpec'

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/stalniy/bdd-lazy-var/issues/14#issuecomment-232495734, or mute the thread https://github.com/notifications/unsubscribe/AAQpaSyHCX96HH7lnuQqhvqoskxOQHcXks5qVVvWgaJpZM4JJkLd .[image: Web Bug from https://github.com/notifications/beacon/AAQpaWvAT6KikFmgVZQzhPagCykxIo4Rks5qVVvWgaJpZM4JJkLd.gif]

dkreft commented 8 years ago

I think you can close this if you want to. I'd really like it if bdd-lazy-var behaved more like rspec but I couldn't think of a way out of the use case you mentioned and I don't have the time to dig into it. :-(

stalniy commented 8 years ago

no worries, I will add extra option

stalniy commented 8 years ago

@dkreft just added commit a30c3ff0565348edc27c41759853b9d6ebc05102 which provides new ui (i.e., rspec style). You can use it as mocha -u bdd-lazy-var/rspec. This is a combination of global ui + rspec variable tracking.

Could you please re-check if it fits all your needs?

dkreft commented 8 years ago

Sorry for the delay. I've just checked this out and it works as expected. Please let me know when you release this to npm so I can get it into my project.

Thanks,

-dan

On Sun, Aug 28, 2016 at 11:56 AM, Sergii Stotskyi notifications@github.com wrote:

@dkreft https://github.com/dkreft just added commit which provides new ui (i.e., rspec style). You can use it as mocha -u bdd-lazy-var/rspec. This is combination of global ui + rspec variable tracking.

Could you please re-check if it fits all your needs?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/stalniy/bdd-lazy-var/issues/14#issuecomment-242992360, or mute the thread https://github.com/notifications/unsubscribe-auth/AAQpactYQssfSCZTJOUk54aKgqobn5JTks5qkdnigaJpZM4JJkLd .[image: Web Bug from https://github.com/notifications/beacon/AAQpaU4FssYaUbw9ofnR9s0-_JoYGFIOks5qkdnigaJpZM4JJkLd.gif] {"api_version":"1.0","publisher":{"api_key":" 05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity": {"external_key":"github/stalniy/bdd-lazy-var","title": "stalniy/bdd-lazy-var","subtitle":"GitHub repository","main_image_url":" https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88- 11e6-95fc-7290892c7bb5.png","avatar_image_url":"https:// cloud.githubusercontent.com/assets/143418/15842166/ 7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/stalniy/bdd-lazy-var"}}," updates":{"snippets":[{"icon":"PERSON","message":"@stalniy in #14: @dkreft just added commit which provides new ui (i.e., rspec style). You can use it as mocha -u bdd-lazy-var/rspec. This is combination of global ui + rspec variable tracking.\r\n\r\nCould you please re-check if it fits all your needs?"}],"action":{"name":"View Issue","url":"https://github. com/stalniy/bdd-lazy-var/issues/14#issuecomment-242992360"}}}

stalniy commented 8 years ago

available in 1.2.0,

Cheers

stalniy commented 6 years ago

Have been thinking about usecase asked in https://github.com/stalniy/bdd-lazy-var/issues/14#issuecomment-232495734 and found 2 solutions for rspec like behavior: