Skip to content
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

Handle failed assertions and include them in reports #86

Closed
rupl opened this issue May 15, 2014 · 32 comments
Closed

Handle failed assertions and include them in reports #86

rupl opened this issue May 15, 2014 · 32 comments
Assignees
Labels
Milestone

Comments

@rupl
Copy link
Contributor

@rupl rupl commented May 15, 2014

I think it would be really great to handle failed assertions and log them the same way a successful run is logged. We could even color-code the data points that fail assertions.

This is the output I see when I set an assertion that fails. In this case I set assert-requests to a very low number in the options object of the Gruntfile:

$ grunt phantomas:requests
Running "phantomas:requests" (phantomas) task

PHANTOMAS EXECUTION(S) STARTED.
>> 5 Phantomas execution(s) done -> checking results:
>> SOMETHING WENT WRONG...
>> TypeError: Cannot call method 'value' of undefined
>> TypeError: Cannot call method 'value' of undefined
>>     at null.<anonymous> (.../grunt/phantomas/node_modules/grunt-phantomas/tasks/lib/phantomas.js:377:50)
>>     at tryCatch2 (.../grunt/phantomas/node_modules/grunt-phantomas/node_modules/bluebird/js/main/util.js:74:19)
>>     at Promise$_resolveFromResolver [as _resolveFromResolver] (.../grunt/phantomas/node_modules/grunt-phantomas/node_modules/bluebird/js/main/promise.js:608:13)
>>     at new Promise (.../grunt/phantomas/node_modules/grunt-phantomas/node_modules/bluebird/js/main/promise.js:88:37)
>>     at Phantomas.formResult (.../grunt/phantomas/node_modules/grunt-phantomas/tasks/lib/phantomas.js:372:10)
>>     at tryCatch1 (.../grunt/phantomas/node_modules/grunt-phantomas/node_modules/bluebird/js/main/util.js:64:19)
>>     at Promise$_callHandler [as _callHandler] (.../grunt/phantomas/node_modules/grunt-phantomas/node_modules/bluebird/js/main/promise.js:708:13)
>>     at Promise$_settlePromiseFromHandler [as _settlePromiseFromHandler] (.../grunt/phantomas/node_modules/grunt-phantomas/node_modules/bluebird/js/main/promise.js:724:18)
>>     at Promise$_settlePromiseAt [as _settlePromiseAt] (.../grunt/phantomas/node_modules/grunt-phantomas/node_modules/bluebird/js/main/promise.js:896:14)
>>     at Promise$_fulfillPromises [as _fulfillPromises] (.../grunt/phantomas/node_modules/grunt-phantomas/node_modules/bluebird/js/main/promise.js:1041:14)

I started hunting around for a solution and noticed that metrics doesn't exist when the assertion fails, which is the undefined reported in the Grunt output. Instead the promise results argument contains a small-ish object with only a couple properties: the Error code (mine was Error 1), and a stack trace.

@stefanjudis
Copy link
Owner

@stefanjudis stefanjudis commented May 15, 2014

Do you mind posting the result returned by phantomas ( can not dig into it right now )?
How it is now and how it could be, to work with it... :)

@macbre has probably something to say about that, too? ;)

@stefanjudis stefanjudis self-assigned this May 15, 2014
@rupl
Copy link
Contributor Author

@rupl rupl commented May 15, 2014

Sure thing. I misspoke earlier and have edited my first post. It's actually the console.log of results before return new Promise..., not the metrics object of the fulfilledPromise. results is an array of objects corresponding to the number of blended runs. Since they're all identical, here is one of the objects inside results:

{ _bitField: 134217728,
    _settledValue:
     { name: 'RejectionError',
       message: '1',
       cause: [Error: 1],
       stack: 'Error: 1\n    at maybeWrapAsError (.../grunt/phantomas/node_modules/grunt-phantomas/node_modules/bluebird/js/main/util.js:128:12)\n    at PromiseResolver$_callback (.../grunt/phantomas/node_modules/grunt-phantomas/node_modules/bluebird/js/main/promise_resolver.js:55:48)\n    at ChildProcess.<anonymous> (.../grunt/phantomas/node_modules/grunt-phantomas/node_modules/phantomas/lib/index.js:118:4)\n    at ChildProcess.EventEmitter.emit (events.js:98:17)\n    at maybeClose (child_process.js:743:16)\n    at Socket.<anonymous> (child_process.js:956:11)\n    at Socket.EventEmitter.emit (events.js:95:17)\n    at Pipe.close (net.js:465:12)' } }
@rupl
Copy link
Contributor Author

@rupl rupl commented May 15, 2014

Here's a Gruntfile you can use to reproduce. Run grunt phantomas:requests

I'm on grunt-phantomas 0.5.3, the copy of Phantomas inside the node_modules dir says version 0.12.1

module.exports = function(grunt) {

  // Project configuration.
  grunt.initConfig({
    pkg: grunt.file.readJSON('package.json'),

    phantomas: {
      default: {
        options: {
          indexPath: './phantomas/',
          options: {},
          url: 'http://gruntjs.com/'
        }
      },
      requests: {
        options: {
          indexPath: './phantomas/',
          options: {
            'assert-requests': 1
          },
          url: 'http://gruntjs.com/'
        }
      },
    }
  });

  grunt.loadNpmTasks('grunt-phantomas');
  grunt.registerTask('default', ['phantomas:default']);
};
@stefanjudis
Copy link
Owner

@stefanjudis stefanjudis commented May 15, 2014

Okay. May I ask you for an update on your side, also?

When we're not on the same version, it will be hard to discuss later on...
But that's already enough for me to dig into it. :)

@rupl
Copy link
Contributor Author

@rupl rupl commented May 15, 2014

No problem, gee I didn't realize I was so far out of date. At a glance it looks pretty close to the same thing. Here's the output of 0.7.0 with console.log(requests) added back in:

$ grunt phantomas:requests
Running "phantomas:requests" (phantomas) task

PHANTOMAS EXECUTION(S) STARTED.
>> 5 Phantomas execution(s) done -> checking results:
[ { _bitField: 134217728,
    _settledValue:
     { name: 'RejectionError',
       message: '1',
       cause: [Error: 1],
       stack: 'Error: 1\n    at maybeWrapAsError (.../grunt/phantomas/node_modules/grunt-phantomas/node_modules/bluebird/js/main/util.js:127:12)\n    at PromiseResolver$_callback (.../grunt/phantomas/node_modules/grunt-phantomas/node_modules/bluebird/js/main/promise_resolver.js:55:48)\n    at ChildProcess.<anonymous> (.../grunt/phantomas/node_modules/grunt-phantomas/node_modules/phantomas/lib/index.js:165:4)\n    at ChildProcess.EventEmitter.emit (events.js:98:17)\n    at maybeClose (child_process.js:743:16)\n    at Socket.<anonymous> (child_process.js:956:11)\n    at Socket.EventEmitter.emit (events.js:95:17)\n    at Pipe.close (net.js:465:12)' } },
  { _bitField: 134217728,
    _settledValue:
     { name: 'RejectionError',
       message: '1',
       cause: [Error: 1],
       stack: 'Error: 1\n    at maybeWrapAsError (.../grunt/phantomas/node_modules/grunt-phantomas/node_modules/bluebird/js/main/util.js:127:12)\n    at PromiseResolver$_callback (.../grunt/phantomas/node_modules/grunt-phantomas/node_modules/bluebird/js/main/promise_resolver.js:55:48)\n    at ChildProcess.<anonymous> (.../grunt/phantomas/node_modules/grunt-phantomas/node_modules/phantomas/lib/index.js:165:4)\n    at ChildProcess.EventEmitter.emit (events.js:98:17)\n    at maybeClose (child_process.js:743:16)\n    at Process.ChildProcess._handle.onexit (child_process.js:810:5)' } },
  { _bitField: 134217728,
    _settledValue:
     { name: 'RejectionError',
       message: '1',
       cause: [Error: 1],
       stack: 'Error: 1\n    at maybeWrapAsError (.../grunt/phantomas/node_modules/grunt-phantomas/node_modules/bluebird/js/main/util.js:127:12)\n    at PromiseResolver$_callback (.../grunt/phantomas/node_modules/grunt-phantomas/node_modules/bluebird/js/main/promise_resolver.js:55:48)\n    at ChildProcess.<anonymous> (.../grunt/phantomas/node_modules/grunt-phantomas/node_modules/phantomas/lib/index.js:165:4)\n    at ChildProcess.EventEmitter.emit (events.js:98:17)\n    at maybeClose (child_process.js:743:16)\n    at Socket.<anonymous> (child_process.js:956:11)\n    at Socket.EventEmitter.emit (events.js:95:17)\n    at Pipe.close (net.js:465:12)' } },
  { _bitField: 134217728,
    _settledValue:
     { name: 'RejectionError',
       message: '1',
       cause: [Error: 1],
       stack: 'Error: 1\n    at maybeWrapAsError (.../grunt/phantomas/node_modules/grunt-phantomas/node_modules/bluebird/js/main/util.js:127:12)\n    at PromiseResolver$_callback (.../grunt/phantomas/node_modules/grunt-phantomas/node_modules/bluebird/js/main/promise_resolver.js:55:48)\n    at ChildProcess.<anonymous> (.../grunt/phantomas/node_modules/grunt-phantomas/node_modules/phantomas/lib/index.js:165:4)\n    at ChildProcess.EventEmitter.emit (events.js:98:17)\n    at maybeClose (child_process.js:743:16)\n    at Socket.<anonymous> (child_process.js:956:11)\n    at Socket.EventEmitter.emit (events.js:95:17)\n    at Pipe.close (net.js:465:12)' } },
  { _bitField: 134217728,
    _settledValue:
     { name: 'RejectionError',
       message: '1',
       cause: [Error: 1],
       stack: 'Error: 1\n    at maybeWrapAsError (.../grunt/phantomas/node_modules/grunt-phantomas/node_modules/bluebird/js/main/util.js:127:12)\n    at PromiseResolver$_callback (.../grunt/phantomas/node_modules/grunt-phantomas/node_modules/bluebird/js/main/promise_resolver.js:55:48)\n    at ChildProcess.<anonymous> (.../grunt/phantomas/node_modules/grunt-phantomas/node_modules/phantomas/lib/index.js:165:4)\n    at ChildProcess.EventEmitter.emit (events.js:98:17)\n    at maybeClose (child_process.js:743:16)\n    at Socket.<anonymous> (child_process.js:956:11)\n    at Socket.EventEmitter.emit (events.js:95:17)\n    at Pipe.close (net.js:465:12)' } } ]
>> Phantomas execution not successful -> RejectionError: 1
>> Phantomas execution not successful -> RejectionError: 1
>> Phantomas execution not successful -> RejectionError: 1
>> Phantomas execution not successful -> RejectionError: 1
>> Phantomas execution not successful -> RejectionError: 1
>> SOMETHING WENT WRONG...
>> No run was successful.
@macbre
Copy link

@macbre macbre commented May 15, 2014

@stefanjudis, correct me if I'm wrong, but the current version of phantomas-grunt does not use multiple runs feature from phantomas. Maybe it's worth porting phantomas-grunt to it (when all metrics stats are available on phantomas side).

@stefanjudis
Copy link
Owner

@stefanjudis stefanjudis commented May 15, 2014

@macbre jap, you're right.

I tried but am depending on this one macbre/phantomas#285.
As far as I remember. :)

@stefanjudis
Copy link
Owner

@stefanjudis stefanjudis commented May 15, 2014

@macbre
Copy link

@macbre macbre commented May 17, 2014

Done :)

@rupl
Copy link
Contributor Author

@rupl rupl commented May 28, 2014

Also of interest to this discussion: https://github.com/gmetais/grunt-devperf/

@stefanjudis
Copy link
Owner

@stefanjudis stefanjudis commented May 28, 2014

👍

probably next week I've got time to tackle this issue. :)

@stefanjudis stefanjudis added this to the v0.8.0 milestone Jun 28, 2014
@stefanjudis
Copy link
Owner

@stefanjudis stefanjudis commented Jun 28, 2014

@rupl
Okay. I'm almost there. One question popped up.

How to deal with failing assertion depending on data type ( median, max, min, ... ).

Currently a warning is shown when one value out of 5 hit the border, but the graph will probably show a circle below the border, when in median mode.

So I'm thinking to make the warnings on top only to show up when median value failed the assertion?
Additionally it's the initial state, so in my mind it makes sense.

What should be the value to show warnings? Max/Min seem to be useless to me, so I'd go for median? what do you think?

@stefanjudis
Copy link
Owner

@stefanjudis stefanjudis commented Jun 28, 2014

screenshot 2014-06-28 18 30 52

stefanjudis added a commit that referenced this issue Jun 28, 2014
@rupl
Copy link
Contributor Author

@rupl rupl commented Jun 28, 2014

Wow, fantastic job on this thing! 🎉

I agree with you that median is the best choice for determining pass/fail. That's that the graph plots, so it makes sense to me.

@stefanjudis
Copy link
Owner

@stefanjudis stefanjudis commented Jun 28, 2014

👍

Cool. I'm just doing some polishing and coverage increasements, but I think could be shipped monday or something. ;)

@stefanjudis
Copy link
Owner

@stefanjudis stefanjudis commented Jun 28, 2014

What do you think of that screenshot (all colors have to be adjusted)?
I'm playing around with really marking failed graphs, but I'm not sure if it's too much?

screenshot 2014-06-28 20 50 27

@rupl
Copy link
Contributor Author

@rupl rupl commented Jun 28, 2014

I think having a toggle to do that would be great. Especially useful if someone displayed the reports on a TV or other big monitor somewhere. But yeah, it might be too much for someone sitting in front of their computer.

@stefanjudis
Copy link
Owner

@stefanjudis stefanjudis commented Jun 28, 2014

Or maybe more something like this?

screenshot 2014-06-28 20 54 12

@stefanjudis
Copy link
Owner

@stefanjudis stefanjudis commented Jun 28, 2014

I think having a toggle to do that would be great.

Let's make a follow up then. ;)

@rupl
Copy link
Contributor Author

@rupl rupl commented Jun 28, 2014

ah schön! the outline a nice balance while still making the failures noticable

@stefanjudis
Copy link
Owner

@stefanjudis stefanjudis commented Jun 28, 2014

Here we are. ;)

http://stefanjudis.github.io/grunt-phantomas/gruntjs/

Release planned definitely monday/tuesday. Now it's 🍺 time. :)

stefanjudis added a commit that referenced this issue Jun 28, 2014
@rupl
Copy link
Contributor Author

@rupl rupl commented Jun 28, 2014

You are amazing. Enjoy 🍻!

@davidlinse
Copy link

@davidlinse davidlinse commented Jun 28, 2014

Great work.. ! 👍

Two things came to my mind while looking at it..

First it would be really nice to be able to initially hide all the tables. At least i want to quick scan all the graphs to get an overview and investigate further (using the tables) if something shows up.

Which leads me to the second thing.. It think it would be really helpful to show a scale on the left/right side of a graph AND scale it up to the largest value.
A graph like:

image

is really hard to interpret without hovering a dot to see a tooltip.
What do you think ?

best regards
~david

@rupl
Copy link
Contributor Author

@rupl rupl commented Jun 28, 2014

I think those are good ideas, would you mind filing a separate issue? Individual scaling for graphs was in the back of my head too.

@stefanjudis
Copy link
Owner

@stefanjudis stefanjudis commented Jun 28, 2014

Thanks guys for feedback.

Jap, let's please make small issues and discuss if needed there. :)

🍻

stefanjudis added a commit that referenced this issue Jun 29, 2014
stefanjudis added a commit that referenced this issue Jun 29, 2014
stefanjudis added a commit that referenced this issue Jun 30, 2014
stefanjudis added a commit that referenced this issue Jun 30, 2014
…ling

added assertion handling - fixes #86
@stefanjudis
Copy link
Owner

@stefanjudis stefanjudis commented Jun 30, 2014

@rupl @davidlinse merged and released. :bowtie:

@davidlinse
Copy link

@davidlinse davidlinse commented Jun 30, 2014

👍

@stefanjudis
Copy link
Owner

@stefanjudis stefanjudis commented Jul 1, 2014

@davidlinse @rupl

And by the way guys,

I'm planning to marry phantomas with psi in a seperate project without grunt dependency.
And collaborators would be awesome... :)

@rupl
Copy link
Contributor Author

@rupl rupl commented Jul 1, 2014

oh man that would be awesome, looking forward to seeing it!

@stefanjudis
Copy link
Owner

@stefanjudis stefanjudis commented Jul 1, 2014

Come on. Say "I'm in." :)

But no pressure. :bowtie:

@rupl
Copy link
Contributor Author

@rupl rupl commented Jul 1, 2014

I'm in 😁

@stefanjudis
Copy link
Owner

@stefanjudis stefanjudis commented Jul 1, 2014

Nice... I'm planning to kick it off in 2-4 weeks.
And I'm fine by removing grunt by myself. After that I'd need some supports with tickets, ideas and interface.

You'll receive an collaboration invite in a sec. :)

Name is also not fixed, yet. But I really like that name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants