This repository has been archived by the owner. It is now read-only.

Deferred tests don't work in BDD mode #55

Closed
andrewdeandrade opened this Issue Jan 7, 2012 · 12 comments

Comments

Projects
None yet
4 participants
@andrewdeandrade

andrewdeandrade commented Jan 7, 2012

This works:

buster.testCase("Pending test example", {
    "//test pending": function () {
    }
})

This doesn't work:

buster.spec.expose();
describe("Pending test example", function() {
  it("//should be pending", function() {

  });
});

On a related not, the Jasmine approach to pending tests is xit() instead of it(). It may be good to alias function to make it easy for people to move from jasmine to buster.

@ghost ghost assigned cjohansen Jan 7, 2012

@augustl

This comment has been minimized.

Member

augustl commented Jan 7, 2012

Assigning Chris, and thanks for the suggestion! Makes sense to model Jasmine in order to conform to the principle of least surprise.

@cjohansen

This comment has been minimized.

Member

cjohansen commented Jan 7, 2012

Currently, it's itEventually(...)

@augustl

This comment has been minimized.

Member

augustl commented Jan 7, 2012

What do you think about having both xit and itEventually?

Or replacing itEventually with xit.

I don't really have an opinion.. I guess we should get a BDD expert on the buster team :)

@andrewdeandrade

This comment has been minimized.

andrewdeandrade commented Jan 7, 2012

I looked at how other libs handle BDD pending tasks and here are some examples

In ScalaTest, they replace the function with a (pending) statement

import org.scalatest.Spec
class ExampleSpec extends Spec {
  describe("A Stack") {
    it("should pop values in last-in-first-out order") (pending)
    it("should throw NoSuchElementException if an empty stack is popped") (pending)
  }

In RSpec, they simply omit the do end block. I reckon this approach could work, since we'd just omit the second argument in the it(testDescription, testFunction). The only downside to this approach is that it doesn't leave room for leaving commented out code in a pending test.

describe "Test some object with pending tests" do
  it "should make sure 2 + 2 = 4" do
    4.should == (2 + 2)
  end 
  it "should make sure the second derivative of something calcs fine"
end

Jasmine uses the approach I described above. NSpec for .NET uses the same xit approach as jasmine.

Vow.js simply replaces the function with a string that says pending:

vows.describe('Array').addBatch({                      // Batch
    'An array': {                                      // Context
        'with 3 elements': {                           // Sub-Context
            topic: [1, 2, 3],                          // Topic
            'has a length of 3': 'pending'
        }
    }
});

Here was an interesting pull request submitted to vows.js, that suggested that pending functions "throw" a pending:
vowsjs/vows#165

TBH, looking at everything out there, TIMTOWTDI, and I don't know if there really is such thing as a BDD "expert"

Personally, I'm in favor of the principle of least surprise and would opt for something that is backward compatible with the existing popular testing libs. Going with both xit and itEventually seems like a reasonable compromise.

I don't know how popular vows.js is, but I know that Jasmine has become the popular client-side BDD framework. At least among the Rails community.

@augustl

This comment has been minimized.

Member

augustl commented Jan 7, 2012

Useful feedback, thanks!

it("should do stuff") with no second argument actually works today I think. it("should to stuff", "some msg") too. The // syntax and itEventually is mostly meant for "commenting out" existing tests.

This doesn't seem to be documented though.

@cjohansen

This comment has been minimized.

Member

cjohansen commented Jan 7, 2012

Documentation is out of date, but these all work todayn

it("does something later");
it("does something later", "when we know more about it");
itEventually("does something");
itEventually("does something", function () { /* ... */ });

Also, xunit:

"do it later": "when we know more about it",
"//do it later": "when we know more about it",
"//do it later": function(){}

I think API compatibility with Jasmine definitely is a worthwhile goal, and don't mind aliases for these features.

@geddski

This comment has been minimized.

geddski commented Jan 23, 2012

I noticed this same thing. I was surprised tonight when this didn't work:

describe('rotate', function(){
  it('// should set SVG rotation', function(){});
});

I'd also expect describe('// rotate'...) to work, but not sure if you can defer an entire test case/describe.

I think it's more important that buster be consistent with itself (BDD and TDD versions) than with other libs. Any inconsistency should be considered a bug IMO.

@geddski

This comment has been minimized.

geddski commented Jan 23, 2012

and FWIW, I think itEventually is very readable and nice too.

@augustl

This comment has been minimized.

Member

augustl commented Jan 23, 2012

Personally, I agree. It makes sense that the handling of "// foo" happens one level lower, the level that handles the shared bdd and xunit data structures, so we get the same behavior on both.

@cjohansen

This comment has been minimized.

Member

cjohansen commented Jan 23, 2012

It makes sense that the handling of "// foo" happens one level lower, the level that handles the shared bdd and xunit data structures

There's no common handling of these - they are two different implementations that both produce the same data type. The data is consumed by the runner, and I don't think it's a good idea for the runner to do heuristics on test names (it would make it impossible to not have that feature).

Anyway, that's more of a pedantic point, I'd be happy to add support for // in BDD names as well.

@cjohansen

This comment has been minimized.

Member

cjohansen commented Mar 4, 2012

Added support for // in BDD spec names: busterjs/buster-test@2a20f2a

@cjohansen cjohansen closed this Mar 4, 2012

@geddski

This comment has been minimized.

geddski commented Mar 5, 2012

nice!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.