xit should set test state to skipped #596

Closed
Iristyle opened this Issue Oct 3, 2012 · 3 comments

Projects

None yet

4 participants

@Iristyle
Iristyle commented Oct 3, 2012

Running under Testacular, which has a small wrapper function in it's Mocha adapter. It uses this to standardize output from Jasmine, Mocha, Angular-Scenario, etc.

I could just use test.pending to set skipped to true... but wondering if perhaps test.state should be set to "skipped" by Mocha.

runner.on('test end', function(test) {
      var result = {
        id: '',
        description: test.title,
        suite: [],
        success: test.state === 'passed',
        skipped: false,
        time: test.duration,
        log: test.$errors || []
      };

      var pointer = test.parent;
      while (!pointer.root) {
        result.suite.unshift(pointer.title);
        pointer = pointer.parent;
      }

      tc.result(result);
    });

Before I do a pull, was looking for thoughts since I'm new to Mocha, etc.

@jugglinmike
Contributor

I'd like to bring this issue up again. Information on skipped tests is relevant when the tests are run. If reporters can't display this info, this API offers no benefit over commenting out tests.

I'd be happy to take a stab at the implementation, but I'd appreciate a thumbs up from @visionmedia first.

@tj
Contributor
tj commented Feb 19, 2013

yeah we could introduce another state maybe, however xit etc will be removed in the future in favour of the it.skip( etc that we have now which is a bit more obvious

@danielstjules
Contributor

Looks like the mocha adapter had been checking test.pending: https://github.com/karma-runner/karma-mocha/blob/7da4e4fc752eaed9f86540ef741cc1790043c2c1/src/adapter.js#L78 It's a good indicator:

$ cat example.js
it('test', function() {});
xit('test2', function() {});
it('test3');

after(function() {
  console.log(this.test.parent.tests);
});

$ ./bin/mocha example.js

  ․․․[ {
    "title": "test",
    "async": 0,
    "sync": true,
    "timedOut": false,
    "pending": false,
    "type": "test",
    "file": "/Users/danielstjules/git/mocha/example.js",
    "parent": "#<Suite>",
    "ctx": "#<Context>",
    "duration": 0,
    "state": "passed",
    "speed": "fast"
  },
  {
    "title": "test2",
    "sync": true,
    "timedOut": false,
    "pending": true,
    "type": "test",
    "file": "/Users/danielstjules/git/mocha/example.js",
    "parent": "#<Suite>",
    "ctx": "#<Context>"
  },
  {
    "title": "test3",
    "sync": true,
    "timedOut": false,
    "pending": true,
    "type": "test",
    "file": "/Users/danielstjules/git/mocha/example.js",
    "parent": "#<Suite>",
    "ctx": "#<Context>"
  } ]

  1 passing (5ms)
  2 pending

To make pending or skipped a state, we'd require a core change that would break backwards compatibility:

function Test(title, fn) {
  Runnable.call(this, title, fn);
  this.pending = !fn;
  this.type = 'test';
}

// =>

function Test(title, fn) {
  Runnable.call(this, title, fn);
  if (!fn) this.state = 'pending';
  this.type = 'test';
}

I don't think it's worth it, even for the purpose of making it more consistent, when the necessary information is already available (through test.pending).

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