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

feat(test): Represent uncaught errors #14513

Merged
merged 17 commits into from
May 9, 2022

Conversation

nayeemrmn
Copy link
Collaborator

@nayeemrmn nayeemrmn commented May 6, 2022

Closes #14449.

Deno.test("failing test", () => {
  throw new Error("message");
});

Deno.test("passing test", () => {});

Deno.test("test with dangling rejection", () => {
  Promise.reject(new Error("foo 3 message"));
});

image
cc @bartlomieju

Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice!

cli/lsp/testing/execution.rs Outdated Show resolved Hide resolved
cli/tools/test.rs Outdated Show resolved Hide resolved
Comment on lines +812 to +841
fn report_uncaught_error(&mut self, origin: &str, js_error: &JsError) {
if self.current_origin == Some(origin.to_string()) {
self.current_origin = None;
}
let stack = self.stack.remove(origin).unwrap_or_default();
let err_string = format!(
"Uncaught error from {}: {}\nThis error was not caught from a test and caused the test runner to fail on the referenced module.\nIt most likely originated from a dangling promise, event/timeout handler or top-level code.",
origin,
test::format_test_error(js_error)
);
let messages = as_test_messages(err_string, false);
for t in stack.iter().rev() {
match t {
TestOrTestStepDescription::TestDescription(desc) => {
self.progress(lsp_custom::TestRunProgressMessage::Failed {
test: desc.into(),
messages: messages.clone(),
duration: None,
});
}
TestOrTestStepDescription::TestStepDescription(desc) => {
self.progress(lsp_custom::TestRunProgressMessage::Failed {
test: desc.into(),
messages: messages.clone(),
duration: None,
});
}
}
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did the lsp part by just making it fail any running test from that origin. cc @kitsonk

@bartlomieju
Copy link
Member

@nayeemrmn I tested this PR in VSCode, but I don't see a difference between this build and latest Deno, all I get is a prompt like this:
Screenshot 2022-05-07 at 16 47 13

@nayeemrmn
Copy link
Collaborator Author

@nayeemrmn I tested this PR in VSCode, but I don't see a difference between this build and latest Deno, all I get is a prompt like this:
Screenshot 2022-05-07 at 16 47 13

I had forgotten to update the worker orchestration on the lsp side, it's done now.

Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bartlomieju bartlomieju merged commit 23efc4f into denoland:main May 9, 2022
@nayeemrmn nayeemrmn deleted the test-uncaught-errors branch May 9, 2022 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Errors in tests still contain internal stack frames
2 participants