Skip to content

Loading…

Improve test error readability #255

Merged
merged 5 commits into from

3 participants

@avital
Meteor Development Group member

I'll run the tests on all the browsers.

@avital avital was assigned
@possibilities

@avital

Knowing what I know now I'm a little embarrassed about my initial shot at this.

This accomplishes the original goal of making the output a little bit easier on the eye without assuming certain keys are present in the fail object.

Thanks,
Mike

@possibilities

Oh right, new before and after given these failing tests

test.equal(5, 6, 'snap!');
test.isNull(1);
test.length([], 2);
moof();

Before and After

@avital avital commented on an outdated diff
packages/test-in-browser/driver.js
@@ -169,13 +169,34 @@ Template.event.events = {
};
Template.event.get_details = function() {
+
+ var prepare = function(details) {
+ return _.compact(_.map(details, function(val, key) {
+ if (!_.isUndefined(val))
@avital Meteor Development Group member
avital added a note

Do you know in which cases val is undefined here? If so can you add a comment for what those cases are?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@avital avital commented on an outdated diff
packages/test-in-browser/driver.html
@@ -74,8 +74,12 @@
{{/if}}
{{#with get_details}}
{{#if this}}
- — {{this}}
+ {{#if type}}— {{type}}{{/if}}
@avital Meteor Development Group member
avital added a note

I know this isn't directly related to your change but can you please comment what the values of {{type}} can be here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@avital avital commented on an outdated diff
packages/test-in-browser/driver.css
@@ -103,7 +103,10 @@
.test_table .event .fail { color: #d00; }
.test_table .event .exception { color: #d00; }
.test_table .event .nodata { color: #ccc; font-style: italic; }
-.test_table .event .xtimes { font-weight: bold; }
+
+.test_table .event .xtimes,
+.test_table .event .failkey
@avital Meteor Development Group member
avital added a note

Can you please follow the style of the other CSS rules on this page? That is -- one line statements when possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@possibilities

Addressed all of your comments.

@avital avital merged commit a7bbb8c into meteor:devel
@avital
Meteor Development Group member

Finally got to this. There was a small bug when get_details runs the second time. Fixed on 62b5f1c.

@glasser
Meteor Development Group member
@possibilities
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 48 additions and 5 deletions.
  1. +2 −1 packages/test-in-browser/driver.css
  2. +19 −1 packages/test-in-browser/driver.html
  3. +27 −3 packages/test-in-browser/driver.js
View
3 packages/test-in-browser/driver.css
@@ -103,7 +103,8 @@
.test_table .event .fail { color: #d00; }
.test_table .event .exception { color: #d00; }
.test_table .event .nodata { color: #ccc; font-style: italic; }
-.test_table .event .xtimes { font-weight: bold; }
+
+.test_table .event .xtimes, .test_table .event .failkey { font-weight: bold; }
.test_table .event .debug {
display: none;
View
20 packages/test-in-browser/driver.html
@@ -74,8 +74,26 @@
{{/if}}
{{#with get_details}}
{{#if this}}
- — {{this}}
+ <!--
+ `type` can be any of the following or a
+ custom assertion type
+
+ * assert_equal
+ * instanceOf
+ * throws
+ * true
+ * null
+ * undefined
+ * NaN
+ * include
+ * length
+ -->
+ {{#if type}}&mdash; {{type}}{{/if}}
+ {{#each details}}
+ - <span class="failkey">{{key}}</span> {{val}}
+ {{/each}}
{{/if}}
+ {{#if stack}}<pre>{{stack}}</pre>{{/if}}
{{/with}}
{{#if is_debuggable}}
<span class="debug">[Debug]</span>
View
30 packages/test-in-browser/driver.js
@@ -169,13 +169,37 @@ Template.event.events = {
};
Template.event.get_details = function() {
+
+ var prepare = function(details) {
+ return _.compact(_.map(details, function(val, key) {
+
+ // You can end up with a an undefined value, e.g. using
+ // isNull without providing a message attribute: isNull(1)
+ if (!_.isUndefined(val))
+ return {
+ key: key,
+ val: val
+ };
+ }));
+ };
+
var details = this.details;
+
if (! details) {
return null;
} else {
- // XXX XXX We need something better than stringify!
- // stringify([undefined]) === "[null]"
- return JSON.stringify(details);
+
+ var type = details.type;
+ delete details.type;
+
+ var stack = details.stack;
+ delete details.stack;
+
+ return {
+ type: type,
+ stack: stack,
+ details: prepare(details)
+ };
}
};
Something went wrong with that request. Please try again.