Use JsReporters to replace existing adapters

[flore77]

With JsReporters it is possible to unify all frameworks adapters into one, which would make the code more cleaner and easier to maintain.

As first step would be replacing the existing adapters with the ones from js-reporters, each with a bit of extra code to instantiate the reporter.

For example:

// QUnit reporter
var runner = new JsReporter.QUnitAdapter(QUnit);

// Listen to the standard events
runner.on('testStart', function() {
  ...
});

runner.on('...', ...);

Secondly to abstract that extra code to create a generic registering mechanism so that we can have only one generic reporter for any framework.

What do you think about this?

[flore77]

Below is the description of the issues that have appeared by trying to integrate JsReporters, describing each case in the external-tests.js file.

Name: QUnit 1.21.0
Test framework: Qunit 1.21.0

Our adapter is working well, but it does not return the real amount of tests, which is 133 and our adapter is returning 132.

This is happening, because it exists a test with:

• moduleName: <script id='qunit-unescaped-module'>'module';</script>
• testName: <script id='qunit-unescaped-test'>'test';</script>

And our adapter is not working well, because it is tokenizing the moduleNames after the character > and we will end up with a wrong suite name, which will no match, so this suite (with all its test) will not be added in the recursive structure.

Suggeste solution: update the QUnit adapter in js-reporters to tokenize after (space)>(space).

Name: Mocha 2.4.5
Test framework: Mocha 2.4.5

Problem details:

• File: test/browser/index.js
• Real number of tests: 19
• Number of passed tests: 18
• Number of failed: 4 (Mocha's html reporter displays one of them twice ?!?)

The problem is with tests in which the done callback is called multiple times, for this type of tests, Mocha emits on "test end" a passing test, but then when the done callback is called again, Mocha will fire only the fail event without firing again the "test end" event (which quite make sense).

So, Mocha's html reporter displays the tests as passing and then if the done cb is called again will display the same test as a failed test, so we will finish in the output with the same test 2 times.

What is pretty weird, by calling also the Mocha's default reporter, and setting its prototype, by counting the tests emitted on "test end", the count is 23, maybe it should be 22 (18 + 4), but how I described above one of the tests is displayed twice and maybe that's why.

Suggested solution: The failed test displayed twice is maybe a bug in Mocha's html reporter.

In async tests in which the done callback is called more than 1 time will be firstly emitted as passed and then on the second call as failed, this is how Mocha is dealing with that.

For example, for Jasmine when the done callback is called the test is emitted and then it does not care if the done callback will be called another time.

I think there are 2 options here:

1. JsReporters sets a standard how to deal with this
2. Changing Mocha adapter to listen to the pass, fail, pending events emitted by Mocha instead of test end will solve the problem

Name: Spine 1.6.2
Test framework: Jasmine2

The Jasmine adapter works and everything is ok. 🎉

Name: Spine 1.0.0
Test framework: Jasmine1

JsReporters has no adapter for Jasmine1.

Suggested solution: The Jasmine versions 1.^ are not even available on npm, which denotes that is not popular, I guess it is not used anymore, so I think it is not worth building an adapter for it. If browserstack wants, it could keep the actual one, or drop it.

Name: QUnit 1.0.0 (optional repo)
Test framework: QUnit 1.0.0

Apparently QUnit.config.modules does not exists in this version and our adapter relies heavily on it.

Suggested solution: This version of QUnit is not even available on npm, and how @jzaefferer has suggested:

There's 1.x versions compatible with 1.0.0 testsuites that work fine with js-reporters.

So there is no reason to bother with this old version and it should be removed from testing.

Name: Mocha 1.21.5
Test framework: Mocha 1.21.5

For the test/browser/large.html test file which has 62 tests (60 passing + 2 failing), the current reporter in browserstack returns 63 tests (60 passing + 3 failing), I checked the file and there are indeed only 62 tests and as also Mocha's html reporter is reporting only 62 tests.

Suggested solution: update the data in external-tests.js file.

[jzaefferer]

Name: QUnit 1.21.0
Suggeste solution: update the QUnit adapter in js-reporters to tokenize after (space)>(space).

I don't think that needs further discussion, let's do it.

Name: Mocha 2.4.5

1. JsReporters sets a standard how to deal with this

Afaik QUnit will also somehow fail tests that call assert.async()() more than once (unless a parameter for multiple done()s was passed), so it seems reasonable to suggest to make this behaviour the standard. Would still have to figure out how this should be reported.

1. Changing Mocha adapter to listen to the pass, fail, pending events emitted by Mocha instead of test end will solve the problem

I don't think that's a good idea here, since its not really a bug in our adapter.

Test framework: Jasmine1
If browserstack wants, it could keep the actual one, or drop it.

Let's just leave the current adapter in place. Unless that makes things a lot more complicated.

I agree with the last two as well.

[flore77]

I don't think that's a good idea here, since its not really a bug in our adapter.

Not quite, since if it's an adapter for Mocha, we should adapt on how Mocha works, even it is not bug.

I will study also QUnit behaviour and I'll be back.

[jzaefferer]

Okay

[flore77]

Qunit async control: async( [acceptCallCount ] ), Type: Number, acceptCallCount (default: 1)

Qunit will fail a test only if the callback returned by assert.async() will be called synchronus more times than acceptCallCount.

Example:

// Failed test
test('aa', function(assert) {
  var done = assert.async();
  assert.ok(true);
  done();
  done();
});

// Passed test
test('aa', function(assert) {
    var done = assert.async();
    assert.ok(true);
    done();
    setTimeout(function() {
      done();
    }, 1000);
});

Mocha would fail both tests.

I really don't know, how the standard should be. Any suggestions? I am not also sure, why should a test fail if the done callback is called multiple times, I guess there would be some reasons.

EDIT: Also Mocha emits the test twice, one time as passed, the other time as failed, QUnit emits only the failed test.

[jzaefferer]

That passed test should also fail in QUnit. Can you file an issue against QUnit for that?

QUnit emits only the failed test.

A single emit for the failure seems correct to me.

[flore77]

A single emit for the failure seems correct to me

Yes, it seems fine for me, too. But I don't know how it can be achieved. Let's take the above passing test ( which should be failed), the done callback is called and Qunit thinks everything is ok, this test has passed, but it will not know if the done callback will be called a second time, it would have to wait for an indefinite period of time. If I set the timeout to be after 5 minutes, but after 10 minutes etc. 😕

[flore77]

Can you file an issue against QUnit for that?

Sure, I will file an issue, but I want to know if this can be achieved to emit the test only one time.

[jzaefferer]

No idea, I can't think of a solution either. I guess the Mocha behaviour is okay, for something that is a bug in a test...

[flore77]

Should I still file an issue on the QUnit repo? Maybe after the dicussion with them, it would be more clearer how the standard should be.

[jzaefferer]

Yes, let's file that issue.