Skip to content

Mocha BDD wrapping incorrectly handles pending tests #2127

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

Closed
adamgruber opened this issue May 19, 2016 · 2 comments
Closed

Mocha BDD wrapping incorrectly handles pending tests #2127

adamgruber opened this issue May 19, 2016 · 2 comments

Comments

@adamgruber
Copy link

Meta -

OS: OSX

Selenium Version: 2.53.2, Node.js

Expected Behavior -

The wrapped function inside javascript/node/selenium-webdriver/testing/index.js should correctly return arguments to the wrapped function for pending tests.

Actual Behavior -

In cases of pending tests, the first and only argument is a title string but the wrapper returns a new function with a function as the first argument instead. The title string is discarded entirely. The issue is not immediately apparent as the test run will continue without error. However, the Test object will not have all properties as expected which can cause issues for some advanced reporters.

Steps to reproduce -

Create a test

// test.js
var test = require('selenium-webdriver/testing');
test.it('Pending test should be written');

Run the test via CLI

mocha test.js

The wrapped function will be called with one argument, a string, "Pending test should be written". It will then call this function:

// selenium-webdriver/testing/index.js
function makeAsyncTestFn(fn) {
  var async = fn.length > 0; // if test function expects a callback, its "async"

  var ret = /** @type {function(this: mocha.Context)}*/ (function(done) {
    var runnable = this.runnable();
    var mochaCallback = runnable.callback;
    runnable.callback = function() {
      flow.reset();
      return mochaCallback.apply(this, arguments);
    };

    var testFn = fn.bind(this);
    flow.execute(function controlFlowExecute() {
      return new promise.Promise(function(fulfill, reject) {
        if (async) {
          // If testFn is async (it expects a done callback), resolve the promise of this
          // test whenever that callback says to.  Any promises returned from testFn are
          // ignored.
          testFn(function testFnDoneCallback(err) {
            if (err) {
              reject(err);
            } else {
              fulfill();
            }
          });
        } else {
          // Without a callback, testFn can return a promise, or it will
          // be assumed to have completed synchronously
          fulfill(testFn());
        }
      }, flow);
    }, runnable.fullTitle()).then(seal(done), done);
  });

  ret.toString = function() {
    return fn.toString();
  };

  return ret;
}

The return will be an object, ret with a runnable property that, if called, will fail due to var testFn = fn.bind(this). You cannot call .bind() on a string. There is no obvious error, however, because this all gets wrapped in a promise.

@jleyba
Copy link
Contributor

jleyba commented May 19, 2016

Pending test cases work for me. I've check Selenium 2.53.2 on Node 4.4.4 and 5.11.1

$ npm install mocha selenium-webdriver > /dev/null
$ ./node_modules/mocha/bin/mocha test.js

  - Pending test should be written

  0 passing (6ms)
  1 pending
$ cat test.js
var test = require('selenium-webdriver/testing');
test.it('Pending test should be written');

@jleyba
Copy link
Contributor

jleyba commented May 19, 2016

Sorry, reading comprehension fail. I see the problem.

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

No branches or pull requests

2 participants