qunitjs / js-reporters

📋 Common Reporter Interface (CRI) for JavaScript testing frameworks.
MIT License
60 stars 18 forks source link

Add autoregister function #90

Closed flore77 closed 8 years ago

flore77 commented 8 years ago

This adds an autoregister function in which js-reporters checks for global testing frameworks and registers the respective adapter returning the runner.

jzaefferer commented 8 years ago

Adding a function without tests and not even docs seems fishy. How do we know it works?

flore77 commented 8 years ago

You are right, I will add some unit tests.

flore77 commented 8 years ago

@jzaefferer I tried to create some unit tests, but I am not able to mock QUnitAdapter functions, because internally in the bundle there is something like:

  var QUnitAdapter = function() {
    ...
  };

  function autoRegister() {
    if (QUnit) {
      var runner = new QUnitAdapter(QUnit);
    }
    ....
  }

  var index = {
    QUnitAdapter: QUnitAdapter
    ...
  };

  return index;

Basically I am mocking index.QUnitAdapter, but this is not used inside the module and I don't know how to get to the internal QUnitAdapter to mock it. Any idea for a workaround ? Or how we should test it?

jzaefferer commented 8 years ago

Can you mock QUnit and test if any of its methods get called by the adapter?

jzaefferer commented 8 years ago

Still missing docs. What's here looks good.

flore77 commented 8 years ago

@jzaefferer do you agree with https://github.com/js-reporters/js-reporters/pull/90/commits/2c452e11f437e01d5bb8bea6167a3e33c943fc02?

I also added the docs.

When this function will run in the browser? Will work? Shoud we do:

  if (QUnit || window.QUnit) {
    ...
  }
  ...
jzaefferer commented 8 years ago

I'm not sure about the browser globals, definitely need to test that.

Maybe we can run one of the test suites in PhantomJS? That's relatively easy to get running on CI.

flore77 commented 8 years ago

Which test suite do you mean to run in PhantomJS? And how should I do it, if you can provide some more details. I found mocha-phantomjs to run mocha tests.

jzaefferer commented 8 years ago

That is still on phantomjs 1.x, maybe https://github.com/nathanboktae/mocha-phantomjs-core is a better choice? I can't quite tell what the difference is...

flore77 commented 8 years ago

@jzaefferer I don't know if that package would work since we have also chai and espacially sinon. I will have a better look.

Meanwhile I have managed to run the tests in the browser with Karma and Webpack, which I must admit is pretty difficult to setup. Sinon is making a lot of trouble when bundled with Webpack, I used the 2.0.0-pre version which was ok, but this version does not satisfy some peer dependencies, but anyway they seem to work.

But now i have some problem with GLOBAL which is not defined in the browser. I tried to do something like var gl = GLOBAL || global, but the tests fail anyway with the error GLOBAL is undefined. Why isn't it evaluated to undefined? Maybe Karma runs the tests in strict mode?

flore77 commented 8 years ago

I managed to run the helpers unit tests also in the browser, I have done some tricks to make it work with sinon.

It does not work for the tap-reporter tests, which are using fake timers (more specifically sinon.test I guess) and the tricks don't work for this. We can add them after sinon would make a release of it's second version, which works perfectly with webpack.

The tests are failing due to the new release of Jasmine 2.5.0, I will investigate.

jzaefferer commented 8 years ago

Nice work! Should I review this now, or wait till you resolved the Jasmine issue?

flore77 commented 8 years ago

No, let me fix the Jasmine thing so we can see that everything is ok.

I found the issue, it is something from Jasmine, from their node package, I already opened a pr, https://github.com/jasmine/jasmine-npm/pull/89. There should be some changes also in our repo, I will open a pr with the solution and explanations.

flore77 commented 8 years ago

@jzaefferer please review :smiley:

jzaefferer commented 8 years ago

👍