WIP: Better errors from t.fail, t.notThrows, t.notThrowsAsync #701
Conversation
|
@michaelfig there is a typing mystery here clearly marked with a TODO. Your advice appreciated! |
|
@dckc my intention is for this PR to also work with xs-ses, but I don't yet know how to test that. |
I'm not sure what you mean. If by xs-ses you mean In case it's useful: the main uses of |
yes
I'm not sure I understand. ava-xs already uses our console, since it is the one left as the global |
Yes. Recall there are SES tests that assert that the console in a compartment is distinct from the one in the start compartment and we agreed it wasn't worthwhile to get ava-xs to satisfy these assertions. Apr 10 notes: |
|
See also Agoric/agoric-sdk#2991 which is more directly about the missing deep stacks. |
|
Provoked by, and helps some with Agoric/agoric-sdk#2992 |
There was a problem hiding this comment.
Are we losing the context of the error when we log to console.error instead of t.log? Ava captures messages in the context of a particular test so it can associate them in the final report. Console messages get unconditionally spammed to the console, without that context.
| @@ -29,7 +27,7 @@ const isPromise = maybePromise => | |||
| const logErrorFirst = (func, args, name, logger = defaultLogger) => { | |||
| let result; | |||
| try { | |||
| result = apply(func, undefined, args); | |||
| result = func(...args); | |||
There was a problem hiding this comment.
What’s the motivation for this simplification, or what was the motive for the complication? (I like the simplification)
| fail(message) { | ||
| const err = new Error(message); | ||
| logger('FAILED by t.fail', err); | ||
| return super.fail(message); |
There was a problem hiding this comment.
Today I learned that super works without the enclosing class.
There was a problem hiding this comment.
This actually makes it more E-like, even though E did not provoke the idea. But still, E did help corroborate that it is a good idea.
There's an interesting tension here. Also, in some other PR where @dckc (IIRC) suggested that I switch from console.log to t.log. I still need to get back to that one as well. In both cases, the tension is that Ava's IIUC, none of this should be a problem on XS as |
yes. |
|
As author, I cannot be a reviewer. But please ask me to review anyway. |
erights
changed the title
|
If we try to do this again, we should start a new PR. Closing this one as fatally stale. |
We may want to redo this as part of #2109. It would be a natural fit. |
DO NOT MERGE: Still based on an ancient master
Draft until demo-tests. But by using
yarn linkto link agoric-sdk to ses-ava, I see that it already improved the error reporting problem reported today by @Chris-Hibbert . This improvement is merely that these three methods also report the error to our console, where we have more control. What it not yet addressed is actually reporting deep stacks.