Core: Implement a node stdout reporter #790

Closed
wants to merge 9 commits into
from

Conversation

4 participants
@leobalter
Member

leobalter commented Mar 19, 2015

With this reporter we can use QUnit directly on npm environments:

var QUnit = require( "../dist/QUnit" );

QUnit.reporter();

require( "./unit-tests.js" );

QUnit.load();

The stdout reporter is only activated when called, this way we keep the retrocompatibility.

Log example: https://www.dropbox.com/s/07k3funzdwp0y7r/Screenshot%202015-03-19%2016.18.25.png?dl=0

The reporter function also accepts an object with the options, the only current available options is output:

// Will print a minimal report, with only dots for each assertion,
// some extra information comes on failures to help locate the issue
QUnit.reporter({
  output: "minimal"
});
// Will print a more detailed report, listing each passing assertion.
QUnit.reporter({
  output: "verbose"
});
@scottgonzalez

This comment has been minimized.

Show comment
Hide comment
@scottgonzalez

scottgonzalez Mar 19, 2015

Contributor

Is the use of chalk the only thing specific to node? You might consider changing this from a node stdout reporter to a console reporter.

Contributor

scottgonzalez commented Mar 19, 2015

Is the use of chalk the only thing specific to node? You might consider changing this from a node stdout reporter to a console reporter.

@leobalter

This comment has been minimized.

Show comment
Hide comment
@leobalter

leobalter Mar 20, 2015

Member

The stdout reporter is also using Node's process.stdout and process.error. AFAIK, the use of console.log is limited to one line for each log, which is not what I'm aiming for here.

Maybe, it will be more clear to replace the QUnit.reporter for QUnit.stdout.

My goal in this PR is to provide a full operational reporter for Node, so we may use QUnit without any adapter.

Member

leobalter commented Mar 20, 2015

The stdout reporter is also using Node's process.stdout and process.error. AFAIK, the use of console.log is limited to one line for each log, which is not what I'm aiming for here.

Maybe, it will be more clear to replace the QUnit.reporter for QUnit.stdout.

My goal in this PR is to provide a full operational reporter for Node, so we may use QUnit without any adapter.

@leobalter

This comment has been minimized.

Show comment
Hide comment
@leobalter

leobalter Mar 24, 2015

Member

The stdout reporter may be followed by a TAP reporter. The one at Mocha may be used as reference.

I'll take another look to see if I detach this code from the build, and make it an individual module, so we can set it as an separate module and just require the module when QUnit.stdout is called.

Member

leobalter commented Mar 24, 2015

The stdout reporter may be followed by a TAP reporter. The one at Mocha may be used as reference.

I'll take another look to see if I detach this code from the build, and make it an individual module, so we can set it as an separate module and just require the module when QUnit.stdout is called.

@leobalter leobalter referenced this pull request in JSRocksHQ/harmonic Mar 24, 2015

Closed

ESLint #122

@leobalter

This comment has been minimized.

Show comment
Hide comment
@leobalter

leobalter Mar 24, 2015

Member

The last commit changed this PR to use the stdout reporter feature from an external module. We can turn the module a jQuery project, as you prefer.

Member

leobalter commented Mar 24, 2015

The last commit changed this PR to use the stdout reporter feature from an external module. We can turn the module a jQuery project, as you prefer.

package.json
@@ -31,7 +31,9 @@
"qunit/qunit.css",
"LICENSE.txt"
],
- "dependencies": {},
+ "dependencies": {
+ "chalk": "1.0.0"

This comment has been minimized.

@jzaefferer

jzaefferer Mar 24, 2015

Member

Is this still needed?

@jzaefferer

jzaefferer Mar 24, 2015

Member

Is this still needed?

This comment has been minimized.

@leobalter

leobalter Mar 25, 2015

Member

removing it.

@leobalter

leobalter Mar 25, 2015

Member

removing it.

package.json
@@ -45,6 +47,7 @@
"grunt-qunit-istanbul": "0.4.5",
"grunt-search": "0.1.6",
"load-grunt-tasks": "0.3.0",
+ "qunit-reporter-stdout": "0.1.0",

This comment has been minimized.

@jzaefferer

jzaefferer Mar 24, 2015

Member

If this is only a devDependency, how does QUnit.stdout() actually add any value? Is all we're removing from userland a single require-call?

@jzaefferer

jzaefferer Mar 24, 2015

Member

If this is only a devDependency, how does QUnit.stdout() actually add any value? Is all we're removing from userland a single require-call?

This comment has been minimized.

@leobalter

leobalter Mar 25, 2015

Member

my bad here, it needs to be a dependency, not for development.

@leobalter

leobalter Mar 25, 2015

Member

my bad here, it needs to be a dependency, not for development.

+QUnit.stdout();
+
+// Load QUnit tests
+require( "./logs" );

This comment has been minimized.

@jzaefferer

jzaefferer Mar 26, 2015

Member

How do these tests get access to QUnit? We must be assigning global.QUnit somewhere for that to work, right?

@jzaefferer

jzaefferer Mar 26, 2015

Member

How do these tests get access to QUnit? We must be assigning global.QUnit somewhere for that to work, right?

This comment has been minimized.

@leobalter

leobalter Mar 27, 2015

Member

It will use the QUnit variable assigned above. This current file test/reporter-stdout.js will work as a context module for its following requirements.

@leobalter

leobalter Mar 27, 2015

Member

It will use the QUnit variable assigned above. This current file test/reporter-stdout.js will work as a context module for its following requirements.

This comment has been minimized.

@jzaefferer

jzaefferer Mar 29, 2015

Member

Required files in node don't have access to the scope of the file that is requiring it. So the only way for the required file to have QUnit in scope is for a global (right?). Where does the global come from?

@jzaefferer

jzaefferer Mar 29, 2015

Member

Required files in node don't have access to the scope of the file that is requiring it. So the only way for the required file to have QUnit in scope is for a global (right?). Where does the global come from?

This comment has been minimized.

@scottgonzalez

scottgonzalez Mar 29, 2015

Contributor

See line 3 above.

@scottgonzalez

scottgonzalez Mar 29, 2015

Contributor

See line 3 above.

This comment has been minimized.

@jzaefferer

jzaefferer Mar 29, 2015

Member

That var QUnit shouldn't be accessible in the scope of any other files required here. Its a local variable, not made global in any way.

@jzaefferer

jzaefferer Mar 29, 2015

Member

That var QUnit shouldn't be accessible in the scope of any other files required here. Its a local variable, not made global in any way.

This comment has been minimized.

@jzaefferer

jzaefferer Mar 29, 2015

Member

That's okay, but still doesn't explain why it was working before.

@jzaefferer

jzaefferer Mar 29, 2015

Member

That's okay, but still doesn't explain why it was working before.

This comment has been minimized.

@leobalter

leobalter Apr 8, 2015

Member

as I told before, this looks like an undocumented feature. Node modules seems to be leaking the current file scope to their following required modules.

@leobalter

leobalter Apr 8, 2015

Member

as I told before, this looks like an undocumented feature. Node modules seems to be leaking the current file scope to their following required modules.

This comment has been minimized.

@scottgonzalez

scottgonzalez Apr 8, 2015

Contributor

Are you sure about that? QUnit does some crazy stuff with globals and different environments.

@scottgonzalez

scottgonzalez Apr 8, 2015

Contributor

Are you sure about that? QUnit does some crazy stuff with globals and different environments.

This comment has been minimized.

@leobalter

leobalter Apr 8, 2015

Member

No, I'm not. I'll check this out on an example repo.

@leobalter

leobalter Apr 8, 2015

Member

No, I'm not. I'll check this out on an example repo.

This comment has been minimized.

@jzaefferer

jzaefferer Apr 22, 2015

Member

Looks like the this from outro.js refers in node to global, which we then alias as window, and in exports.js we then assign window.QUnit, which in node is the same as global.QUnit. Let's fix that.

@jzaefferer

jzaefferer Apr 22, 2015

Member

Looks like the this from outro.js refers in node to global, which we then alias as window, and in exports.js we then assign window.QUnit, which in node is the same as global.QUnit. Let's fix that.

@leobalter

This comment has been minimized.

Show comment
Hide comment
@leobalter

leobalter Apr 8, 2015

Member

bumped the reporter module to 0.2.0 where I included output tests. (the minor version was due to some differences on the stderr output on failing tests)

Member

leobalter commented Apr 8, 2015

bumped the reporter module to 0.2.0 where I included output tests. (the minor version was due to some differences on the stderr output on failing tests)

@jzaefferer jzaefferer referenced this pull request Apr 14, 2015

Closed

Pendings tests #787

leobalter added a commit to leobalter/qunit that referenced this pull request Apr 22, 2015

Build: Remove unintended QUnit global export on Node
Ref #790

QUnit was found exported to the global object on Node environment

leobalter added a commit to leobalter/qunit that referenced this pull request Aug 11, 2015

Build: Remove unintended QUnit global export on Node
Ref #790

QUnit was found exported to the global object on Node environment
@leobalter

This comment has been minimized.

Show comment
Hide comment
@leobalter

leobalter Aug 13, 2015

Member

By several arguments, I'm closing this PR.

We are on the way to have a release of js-reporters, as many improvement ideas came after this PR and now rebasing this is not easy.

Member

leobalter commented Aug 13, 2015

By several arguments, I'm closing this PR.

We are on the way to have a release of js-reporters, as many improvement ideas came after this PR and now rebasing this is not easy.

@leobalter leobalter closed this Aug 13, 2015

leobalter added a commit to leobalter/qunit that referenced this pull request Aug 16, 2015

Build: Remove unintended QUnit global export on Node
Ref #790

QUnit was found exported to the global object on Node environment

leobalter added a commit to leobalter/qunit that referenced this pull request Aug 16, 2015

Build: Remove unintended QUnit global export on Node
Ref #790
Closes #813

QUnit was found exported to the global object on Node environment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment