feat(test): skip internal stack frames for errors

[bartlomieju]

This commit changes "deno test" to filter out stack frames if it is beneficial to the user.

This is the case when there are stack frames coming from "internal" code
below frames coming from user code.

Co-Authored-By: Nayeem Rahman [email protected]

[bartlomieju]

@nayeemrmn I'm not sure we can get away without adding new method on PrettyJsError - in this case "cause" and "aggregated" errors still have full stack traces. Maybe that's fine, but it feels we should remove internal stack frames from them too...

[nayeemrmn]

fn abbreviate_test_error(js_error: &JsError) -> JsError {
  let mut js_error = js_error.clone();
  let frames = std::mem::take(&mut js_error.frames);
  let mut frames = frames
    .into_iter()
    .rev()
    .skip_while(|f| {
      if let Some(file_name) = &f.file_name {
        file_name.starts_with("[deno:") || file_name.starts_with("deno:")
      } else {
        false
      }
    })
    .into_iter()
    .collect::<Vec<_>>();
  frames.reverse();
  js_error.frames = frames;
  js_error.cause = js_error
    .cause
    .as_ref()
    .map(|e| Box::new(abbreviate_test_error(e)));
  js_error.aggregated = js_error
    .aggregated
    .as_ref()
    .map(|es| es.iter().map(abbreviate_test_error).collect());
  js_error
}

fn format_test_error(js_error: &JsError) -> String {
  let mut js_error = abbreviate_test_error(js_error);
  let e =
    colors::red(&js_error.exception_message.trim_start_matches("Uncaught "))
      .to_string();
  js_error.exception_message = e;
  PrettyJsError::create(js_error).to_string()
}

I also added a re-reverse for the frames, I don't expect having the upside down stacks was intentional?

[bartlomieju]

@nayeemrmn I applied some more logic for filtering - ie. frames are only filtered if there's at least a single user code frame. Otherwise we could end up with single line errors without any context (eg. errors coming from escalating permissions).

This should now be done.

[lucacasonato]

Otherwise we could end up with single line errors without any context (eg. errors coming from escalating permissions)

We should really try to hook this up so the stack trace points to the Deno.test invocation. (follow up PR)

[bartlomieju]

Otherwise we could end up with single line errors without any context (eg. errors coming from escalating permissions)

We should really try to hook this up so the stack trace points to the Deno.test invocation. (follow up PR)

I'm not sure what you mean, please elaborate

[lucacasonato]

If the permission change fails, the error that is thrown should have the Deno.test invocation in the stack trace (right now the stack trace just points to somewhere in the internal test runner code).

[bartlomieju]

If the permission change fails, the error that is thrown should have the Deno.test invocation in the stack trace (right now the stack trace just points to somewhere in the internal test runner code).

I see, this will require some gymnastics to do it only for permission errors, but should be doable.

[lucacasonato]

Tried it out, and looks to be working well locally. I really dislike the red color of the error message (but not the stack frames). It makes the output much more messy IMO. It'd be great if we could revert that change.

[kewp]

Is there any way to remove the stack frame for user-defined code?

[bartlomieju]

You can try using Error.captureStackTrace

[kewp]

Thanks @bartlomieju I managed to get it to work with your suggestion.

For anyone interested, this is what the code looks like:

try {
    if (!validateTestShape(testMod.default)) fail("test shape is invalid");
}
catch (error) {
    const newError = new AssertionError(error.message);
    Error.captureStackTrace(newError, newError.constructor); // capture the stack trace
    throw newError; // re-throw the error without the stack trace
}