mirror of
https://github.com/denoland/deno.git
synced 2024-12-24 08:09:08 -05:00
fix(lsp): test explorer panic on step result (#20289)
Fixes https://github.com/denoland/vscode_deno/issues/843. Prevents step results from being reported twice. Refactors `LspTestReporter` to use a complete `(test_id, descriptor)` map instead of a brittle `LspTestReporter::stack`.
This commit is contained in:
parent
fd70b7025b
commit
907d9bb4d7
2 changed files with 172 additions and 110 deletions
|
@ -334,6 +334,7 @@ impl TestRun {
|
|||
spawn(async move {
|
||||
let earlier = Instant::now();
|
||||
let mut summary = test::TestSummary::new();
|
||||
let mut tests_with_result = HashSet::new();
|
||||
let mut used_only = false;
|
||||
|
||||
while let Some(event) = receiver.recv().await {
|
||||
|
@ -359,20 +360,21 @@ impl TestRun {
|
|||
reporter.report_output(&output);
|
||||
}
|
||||
test::TestEvent::Result(id, result, elapsed) => {
|
||||
let description = tests.read().get(&id).unwrap().clone();
|
||||
match &result {
|
||||
test::TestResult::Ok => summary.passed += 1,
|
||||
test::TestResult::Ignored => summary.ignored += 1,
|
||||
test::TestResult::Failed(error) => {
|
||||
summary.failed += 1;
|
||||
summary.failures.push((description.clone(), error.clone()));
|
||||
}
|
||||
test::TestResult::Cancelled => {
|
||||
summary.failed += 1;
|
||||
if tests_with_result.insert(id) {
|
||||
let description = tests.read().get(&id).unwrap().clone();
|
||||
match &result {
|
||||
test::TestResult::Ok => summary.passed += 1,
|
||||
test::TestResult::Ignored => summary.ignored += 1,
|
||||
test::TestResult::Failed(error) => {
|
||||
summary.failed += 1;
|
||||
summary.failures.push((description.clone(), error.clone()));
|
||||
}
|
||||
test::TestResult::Cancelled => {
|
||||
summary.failed += 1;
|
||||
}
|
||||
}
|
||||
reporter.report_result(&description, &result, elapsed);
|
||||
}
|
||||
|
||||
reporter.report_result(&description, &result, elapsed);
|
||||
}
|
||||
test::TestEvent::UncaughtError(origin, error) => {
|
||||
reporter.report_uncaught_error(&origin, &error);
|
||||
|
@ -387,22 +389,24 @@ impl TestRun {
|
|||
reporter.report_step_wait(test_steps.get(&id).unwrap());
|
||||
}
|
||||
test::TestEvent::StepResult(id, result, duration) => {
|
||||
match &result {
|
||||
test::TestStepResult::Ok => {
|
||||
summary.passed_steps += 1;
|
||||
}
|
||||
test::TestStepResult::Ignored => {
|
||||
summary.ignored_steps += 1;
|
||||
}
|
||||
test::TestStepResult::Failed(_) => {
|
||||
summary.failed_steps += 1;
|
||||
if tests_with_result.insert(id) {
|
||||
match &result {
|
||||
test::TestStepResult::Ok => {
|
||||
summary.passed_steps += 1;
|
||||
}
|
||||
test::TestStepResult::Ignored => {
|
||||
summary.ignored_steps += 1;
|
||||
}
|
||||
test::TestStepResult::Failed(_) => {
|
||||
summary.failed_steps += 1;
|
||||
}
|
||||
}
|
||||
reporter.report_step_result(
|
||||
test_steps.get(&id).unwrap(),
|
||||
&result,
|
||||
duration,
|
||||
);
|
||||
}
|
||||
reporter.report_step_result(
|
||||
test_steps.get(&id).unwrap(),
|
||||
&result,
|
||||
duration,
|
||||
);
|
||||
}
|
||||
test::TestEvent::Sigint => {}
|
||||
}
|
||||
|
@ -479,6 +483,15 @@ enum TestOrTestStepDescription {
|
|||
TestStepDescription(test::TestStepDescription),
|
||||
}
|
||||
|
||||
impl TestOrTestStepDescription {
|
||||
fn origin(&self) -> &str {
|
||||
match self {
|
||||
TestOrTestStepDescription::TestDescription(d) => d.origin.as_str(),
|
||||
TestOrTestStepDescription::TestStepDescription(d) => d.origin.as_str(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl From<&test::TestDescription> for TestOrTestStepDescription {
|
||||
fn from(desc: &test::TestDescription) -> Self {
|
||||
Self::TestDescription(desc.clone())
|
||||
|
@ -560,11 +573,11 @@ impl From<&test::TestStepDescription> for lsp_custom::TestIdentifier {
|
|||
|
||||
struct LspTestReporter {
|
||||
client: Client,
|
||||
current_origin: Option<String>,
|
||||
maybe_root_uri: Option<ModuleSpecifier>,
|
||||
id: u32,
|
||||
stack: HashMap<String, Vec<TestOrTestStepDescription>>,
|
||||
tests: Arc<Mutex<HashMap<ModuleSpecifier, TestDefinitions>>>,
|
||||
maybe_root_uri: Option<ModuleSpecifier>,
|
||||
files: Arc<Mutex<HashMap<ModuleSpecifier, TestDefinitions>>>,
|
||||
tests: IndexMap<usize, TestOrTestStepDescription>,
|
||||
current_test: Option<usize>,
|
||||
}
|
||||
|
||||
impl LspTestReporter {
|
||||
|
@ -572,15 +585,15 @@ impl LspTestReporter {
|
|||
run: &TestRun,
|
||||
client: Client,
|
||||
maybe_root_uri: Option<&ModuleSpecifier>,
|
||||
tests: Arc<Mutex<HashMap<ModuleSpecifier, TestDefinitions>>>,
|
||||
files: Arc<Mutex<HashMap<ModuleSpecifier, TestDefinitions>>>,
|
||||
) -> Self {
|
||||
Self {
|
||||
client,
|
||||
current_origin: None,
|
||||
maybe_root_uri: maybe_root_uri.cloned(),
|
||||
id: run.id,
|
||||
stack: HashMap::new(),
|
||||
tests,
|
||||
maybe_root_uri: maybe_root_uri.cloned(),
|
||||
files,
|
||||
tests: Default::default(),
|
||||
current_test: Default::default(),
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -598,8 +611,9 @@ impl LspTestReporter {
|
|||
fn report_plan(&mut self, _plan: &test::TestPlan) {}
|
||||
|
||||
fn report_register(&mut self, desc: &test::TestDescription) {
|
||||
let mut tests = self.tests.lock();
|
||||
let tds = tests
|
||||
self.tests.insert(desc.id, desc.into());
|
||||
let mut files = self.files.lock();
|
||||
let tds = files
|
||||
.entry(ModuleSpecifier::parse(&desc.location.file_name).unwrap())
|
||||
.or_default();
|
||||
if tds.inject(desc.into()) {
|
||||
|
@ -626,23 +640,17 @@ impl LspTestReporter {
|
|||
}
|
||||
|
||||
fn report_wait(&mut self, desc: &test::TestDescription) {
|
||||
self.current_origin = Some(desc.origin.clone());
|
||||
let test: lsp_custom::TestIdentifier = desc.into();
|
||||
let stack = self.stack.entry(desc.origin.clone()).or_default();
|
||||
assert!(stack.is_empty());
|
||||
stack.push(desc.into());
|
||||
self.current_test = Some(desc.id);
|
||||
let test = desc.into();
|
||||
self.progress(lsp_custom::TestRunProgressMessage::Started { test });
|
||||
}
|
||||
|
||||
fn report_output(&mut self, output: &[u8]) {
|
||||
let test = self.current_origin.as_ref().and_then(|origin| {
|
||||
self
|
||||
.stack
|
||||
.get(origin)
|
||||
.and_then(|v| v.last().map(|td| td.into()))
|
||||
});
|
||||
let test = self
|
||||
.current_test
|
||||
.as_ref()
|
||||
.map(|id| self.tests.get(id).unwrap().into());
|
||||
let value = String::from_utf8_lossy(output).replace('\n', "\r\n");
|
||||
|
||||
self.progress(lsp_custom::TestRunProgressMessage::Output {
|
||||
value,
|
||||
test,
|
||||
|
@ -657,10 +665,7 @@ impl LspTestReporter {
|
|||
result: &test::TestResult,
|
||||
elapsed: u64,
|
||||
) {
|
||||
let stack = self.stack.entry(desc.origin.clone()).or_default();
|
||||
assert_eq!(stack.len(), 1);
|
||||
assert_eq!(stack.pop(), Some(desc.into()));
|
||||
self.current_origin = None;
|
||||
self.current_test = None;
|
||||
match result {
|
||||
test::TestResult::Ok => {
|
||||
self.progress(lsp_custom::TestRunProgressMessage::Passed {
|
||||
|
@ -691,78 +696,69 @@ impl LspTestReporter {
|
|||
}
|
||||
|
||||
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();
|
||||
self.current_test = None;
|
||||
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::fmt::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,
|
||||
});
|
||||
}
|
||||
}
|
||||
for desc in self.tests.values().filter(|d| d.origin() == origin) {
|
||||
self.progress(lsp_custom::TestRunProgressMessage::Failed {
|
||||
test: desc.into(),
|
||||
messages: messages.clone(),
|
||||
duration: None,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
fn report_step_register(&mut self, desc: &test::TestStepDescription) {
|
||||
let mut tests = self.tests.lock();
|
||||
let tds = tests
|
||||
self.tests.insert(desc.id, desc.into());
|
||||
let mut files = self.files.lock();
|
||||
let tds = files
|
||||
.entry(ModuleSpecifier::parse(&desc.location.file_name).unwrap())
|
||||
.or_default();
|
||||
if tds.inject(desc.into()) {
|
||||
let specifier = ModuleSpecifier::parse(&desc.origin).unwrap();
|
||||
let mut prev: lsp_custom::TestData = desc.into();
|
||||
if let Some(stack) = self.stack.get(&desc.origin) {
|
||||
for item in stack.iter().rev() {
|
||||
let mut data: lsp_custom::TestData = item.into();
|
||||
data.steps = vec![prev];
|
||||
prev = data;
|
||||
let data: lsp_custom::TestData = desc.into();
|
||||
if tds.inject(data.clone()) {
|
||||
let mut step_desc = Some(desc);
|
||||
let mut data = data;
|
||||
while let Some(desc) = step_desc {
|
||||
let parent_desc = self.tests.get(&desc.parent_id).unwrap();
|
||||
let mut parent_data: lsp_custom::TestData = parent_desc.into();
|
||||
parent_data.steps = vec![data];
|
||||
data = parent_data;
|
||||
step_desc = match parent_desc {
|
||||
TestOrTestStepDescription::TestDescription(_) => None,
|
||||
TestOrTestStepDescription::TestStepDescription(d) => Some(d),
|
||||
}
|
||||
let label = if let Some(root) = &self.maybe_root_uri {
|
||||
specifier.as_str().replace(root.as_str(), "")
|
||||
} else {
|
||||
specifier
|
||||
.path_segments()
|
||||
.and_then(|s| s.last().map(|s| s.to_string()))
|
||||
.unwrap_or_else(|| "<unknown>".to_string())
|
||||
};
|
||||
self
|
||||
.client
|
||||
.send_test_notification(TestingNotification::Module(
|
||||
lsp_custom::TestModuleNotificationParams {
|
||||
text_document: lsp::TextDocumentIdentifier { uri: specifier },
|
||||
kind: lsp_custom::TestModuleNotificationKind::Insert,
|
||||
label,
|
||||
tests: vec![prev],
|
||||
},
|
||||
));
|
||||
}
|
||||
let specifier = ModuleSpecifier::parse(&desc.origin).unwrap();
|
||||
let label = if let Some(root) = &self.maybe_root_uri {
|
||||
specifier.as_str().replace(root.as_str(), "")
|
||||
} else {
|
||||
specifier
|
||||
.path_segments()
|
||||
.and_then(|s| s.last().map(|s| s.to_string()))
|
||||
.unwrap_or_else(|| "<unknown>".to_string())
|
||||
};
|
||||
self
|
||||
.client
|
||||
.send_test_notification(TestingNotification::Module(
|
||||
lsp_custom::TestModuleNotificationParams {
|
||||
text_document: lsp::TextDocumentIdentifier { uri: specifier },
|
||||
kind: lsp_custom::TestModuleNotificationKind::Insert,
|
||||
label,
|
||||
tests: vec![data],
|
||||
},
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
fn report_step_wait(&mut self, desc: &test::TestStepDescription) {
|
||||
if self.current_test == Some(desc.parent_id) {
|
||||
self.current_test = Some(desc.id);
|
||||
}
|
||||
let test: lsp_custom::TestIdentifier = desc.into();
|
||||
let stack = self.stack.entry(desc.origin.clone()).or_default();
|
||||
self.current_origin = Some(desc.origin.clone());
|
||||
assert!(!stack.is_empty());
|
||||
stack.push(desc.into());
|
||||
self.progress(lsp_custom::TestRunProgressMessage::Started { test });
|
||||
}
|
||||
|
||||
|
@ -772,8 +768,9 @@ impl LspTestReporter {
|
|||
result: &test::TestStepResult,
|
||||
elapsed: u64,
|
||||
) {
|
||||
let stack = self.stack.entry(desc.origin.clone()).or_default();
|
||||
assert_eq!(stack.pop(), Some(desc.into()));
|
||||
if self.current_test == Some(desc.id) {
|
||||
self.current_test = Some(desc.parent_id);
|
||||
}
|
||||
match result {
|
||||
test::TestStepResult::Ok => {
|
||||
self.progress(lsp_custom::TestRunProgressMessage::Passed {
|
||||
|
|
|
@ -8256,8 +8256,9 @@ fn lsp_testing_api() {
|
|||
let contents = r#"
|
||||
Deno.test({
|
||||
name: "test a",
|
||||
fn() {
|
||||
async fn(t) {
|
||||
console.log("test a");
|
||||
await t.step("step of test a", () => {});
|
||||
}
|
||||
});
|
||||
"#;
|
||||
|
@ -8287,7 +8288,6 @@ Deno.test({
|
|||
assert_eq!(params.tests.len(), 1);
|
||||
let test = ¶ms.tests[0];
|
||||
assert_eq!(test.label, "test a");
|
||||
assert!(test.steps.is_none());
|
||||
assert_eq!(
|
||||
test.range,
|
||||
Some(lsp::Range {
|
||||
|
@ -8301,6 +8301,23 @@ Deno.test({
|
|||
}
|
||||
})
|
||||
);
|
||||
let steps = test.steps.as_ref().unwrap();
|
||||
assert_eq!(steps.len(), 1);
|
||||
let step = &steps[0];
|
||||
assert_eq!(step.label, "step of test a");
|
||||
assert_eq!(
|
||||
step.range,
|
||||
Some(lsp::Range {
|
||||
start: lsp::Position {
|
||||
line: 5,
|
||||
character: 12,
|
||||
},
|
||||
end: lsp::Position {
|
||||
line: 5,
|
||||
character: 16,
|
||||
}
|
||||
})
|
||||
);
|
||||
|
||||
let res = client.write_request_with_res_as::<TestRunResponseParams>(
|
||||
"deno/testRun",
|
||||
|
@ -8368,6 +8385,54 @@ Deno.test({
|
|||
}))
|
||||
);
|
||||
|
||||
let (method, notification) = client.read_notification::<Value>();
|
||||
assert_eq!(method, "deno/testRunProgress");
|
||||
assert_eq!(
|
||||
notification,
|
||||
Some(json!({
|
||||
"id": 1,
|
||||
"message": {
|
||||
"type": "started",
|
||||
"test": {
|
||||
"textDocument": {
|
||||
"uri": specifier,
|
||||
},
|
||||
"id": id,
|
||||
"stepId": step.id,
|
||||
},
|
||||
}
|
||||
}))
|
||||
);
|
||||
|
||||
let (method, notification) = client.read_notification::<Value>();
|
||||
assert_eq!(method, "deno/testRunProgress");
|
||||
let mut notification = notification.unwrap();
|
||||
let duration = notification
|
||||
.as_object_mut()
|
||||
.unwrap()
|
||||
.get_mut("message")
|
||||
.unwrap()
|
||||
.as_object_mut()
|
||||
.unwrap()
|
||||
.remove("duration");
|
||||
assert!(duration.is_some());
|
||||
assert_eq!(
|
||||
notification,
|
||||
json!({
|
||||
"id": 1,
|
||||
"message": {
|
||||
"type": "passed",
|
||||
"test": {
|
||||
"textDocument": {
|
||||
"uri": specifier,
|
||||
},
|
||||
"id": id,
|
||||
"stepId": step.id,
|
||||
},
|
||||
}
|
||||
})
|
||||
);
|
||||
|
||||
let (method, notification) = client.read_notification::<Value>();
|
||||
assert_eq!(method, "deno/testRunProgress");
|
||||
let notification = notification.unwrap();
|
||||
|
|
Loading…
Reference in a new issue