Repository: denoland/deno_core
Pull request #365 (2023-12-03 16:12:43)

test(core): add more intense testing around promises
These tests wiil be used in #359 but we can land them beforehand. One test returns an unexpected result.


Patch:
diff --git a/Cargo.lock b/Cargo.lock
index b32d559..f83df67 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -273,6 +273,7 @@ dependencies = [
  "parking_lot",
  "pin-project",
  "pretty_assertions",
+ "rstest",
  "serde",
  "serde_json",
  "serde_v8",
@@ -505,6 +506,12 @@ version = "0.3.29"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "efd193069b0ddadc69c46389b740bbccdd97203899b48d09c5f7969591d6bae2"
 
+[[package]]
+name = "futures-timer"
+version = "3.0.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "e64b03909df88034c26dc1547e8970b91f98bdb65165d6a4e9110d94263dbb2c"
+
 [[package]]
 name = "futures-util"
 version = "0.3.29"
@@ -1018,6 +1025,35 @@ version = "1.9.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "c707298afce11da2efef2f600116fa93ffa7a032b5d7b628aa17711ec81383ca"
 
+[[package]]
+name = "rstest"
+version = "0.18.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "97eeab2f3c0a199bc4be135c36c924b6590b88c377d416494288c14f2db30199"
+dependencies = [
+ "futures",
+ "futures-timer",
+ "rstest_macros",
+ "rustc_version 0.4.0",
+]
+
+[[package]]
+name = "rstest_macros"
+version = "0.18.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d428f8247852f894ee1be110b375111b586d4fa431f6c46e64ba5a0dcccbe605"
+dependencies = [
+ "cfg-if",
+ "glob",
+ "proc-macro2",
+ "quote",
+ "regex",
+ "relative-path",
+ "rustc_version 0.4.0",
+ "syn 2.0.39",
+ "unicode-ident",
+]
+
 [[package]]
 name = "rustc-demangle"
 version = "0.1.23"
diff --git a/Cargo.toml b/Cargo.toml
index e3e3853..ec1f0b9 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -39,6 +39,7 @@ pin-project = "1"
 pretty_assertions = "1.3.0"
 rand = "0.8.5"
 prettyplease = "0.2.15"
+rstest = "0"
 serde = { version = "1", features = ["derive"] }
 serde_bytes = "0.11"
 serde_json = "1"
diff --git a/core/Cargo.toml b/core/Cargo.toml
index a11eff9..5573805 100644
--- a/core/Cargo.toml
+++ b/core/Cargo.toml
@@ -45,6 +45,7 @@ cooked-waker.workspace = true
 deno_ast.workspace = true
 bencher.workspace = true
 pretty_assertions.workspace = true
+rstest.workspace = true
 
 [[bench]]
 name = "ops_sync"
diff --git a/core/mod.js b/core/mod.js
new file mode 100644
index 0000000..a7221f9
--- /dev/null
+++ b/core/mod.js
@@ -0,0 +1,6 @@
+// Re-export fields from `globalThis.__bootstrap` so that embedders using
+// ES modules can import these symbols instead of capturing the bootstrap ns.
+const bootstrap = globalThis.__bootstrap;
+const { core, internals, primordials } = bootstrap;
+
+export { core, internals, primordials };
diff --git a/core/modules/map.rs b/core/modules/map.rs
index 59d7bcb..45b33cc 100644
--- a/core/modules/map.rs
+++ b/core/modules/map.rs
@@ -23,6 +23,7 @@ use crate::runtime::JsRuntimeState;
 use crate::runtime::SnapshottedData;
 use crate::JsRuntime;
 use crate::ModuleSource;
+use crate::ModuleSpecifier;
 use anyhow::bail;
 use anyhow::Error;
 use futures::future::FutureExt;
@@ -1301,6 +1302,58 @@ impl ModuleMap {
 
     vec![]
   }
+
+  /// Load and evaluate an ES module provided the specifier and source code.
+  ///
+  /// The module should not have Top-Level Await (that is, it should be
+  /// possible to evaluate it synchronously).
+  ///
+  /// It is caller's responsibility to ensure that not duplicate specifiers are
+  /// passed to this method.
+  pub(crate) fn lazy_load_es_module_from_code(
+    &self,
+    scope: &mut v8::HandleScope,
+    module_specifier: &str,
+    source_code: ModuleCode,
+  ) -> Result<v8::Global<v8::Value>, Error> {
+    let specifier = ModuleSpecifier::parse(module_specifier)?;
+    let mod_id = self
+      .new_es_module(scope, false, specifier.into(), source_code, false)
+      .map_err(|e| match e {
+        ModuleError::Exception(exception) => {
+          let exception = v8::Local::new(scope, exception);
+          exception_to_err_result::<()>(scope, exception, false).unwrap_err()
+        }
+        ModuleError::Other(error) => error,
+      })?;
+
+    self.instantiate_module(scope, mod_id).map_err(|e| {
+      let exception = v8::Local::new(scope, e);
+      exception_to_err_result::<()>(scope, exception, false).unwrap_err()
+    })?;
+
+    let module_handle = self.get_handle(mod_id).unwrap();
+    let module_local = v8::Local::<v8::Module>::new(scope, module_handle);
+
+    let status = module_local.get_status();
+    assert_eq!(status, v8::ModuleStatus::Instantiated);
+
+    let value = module_local.evaluate(scope).unwrap();
+    let promise = v8::Local::<v8::Promise>::try_from(value).unwrap();
+    let result = promise.result(scope);
+    if !result.is_undefined() {
+      return Err(
+        exception_to_err_result::<()>(scope, result, false).unwrap_err(),
+      );
+    }
+
+    let status = module_local.get_status();
+    assert_eq!(status, v8::ModuleStatus::Evaluated);
+
+    let mod_ns = module_local.get_module_namespace();
+
+    Ok(v8::Global::new(scope, mod_ns))
+  }
 }
 
 impl Default for ModuleMap {
diff --git a/core/modules/module_map_data.rs b/core/modules/module_map_data.rs
index 96356a8..5258de0 100644
--- a/core/modules/module_map_data.rs
+++ b/core/modules/module_map_data.rs
@@ -491,10 +491,11 @@ impl ModuleMapData {
   #[cfg(test)]
   pub fn assert_module_map(&self, modules: &Vec<ModuleInfo>) {
     let data = self;
-    assert_eq!(data.handles.len(), modules.len());
-    assert_eq!(data.info.len(), modules.len());
+    // There's always one internal `deno_core` ES module loaded, so +1 here.
+    assert_eq!(data.handles.len(), modules.len() + 1);
+    assert_eq!(data.info.len(), modules.len() + 1);
     assert_eq!(data.next_load_id as usize, modules.len());
-    assert_eq!(data.by_name.len(), modules.len());
+    assert_eq!(data.by_name.len(), modules.len() + 1);
 
     for info in modules {
       assert!(data.handles.get(info.id).is_some());
diff --git a/core/runtime/jsruntime.rs b/core/runtime/jsruntime.rs
index df04605..007e841 100644
--- a/core/runtime/jsruntime.rs
+++ b/core/runtime/jsruntime.rs
@@ -221,6 +221,8 @@ pub(crate) const BUILTIN_SOURCES: [ExtensionFileSource; 3] = include_js_files!(
   "01_core.js",
   "02_error.js",
 );
+pub(crate) const BUILTIN_ES_MODULES: [ExtensionFileSource; 1] =
+  include_js_files!(core "mod.js",);
 
 /// A single execution context of JavaScript. Corresponds roughly to the "Web
 /// Worker" concept in the DOM.
@@ -919,6 +921,14 @@ impl JsRuntime {
             file_source.load()?,
           )?;
         }
+        for file_source in &BUILTIN_ES_MODULES {
+          let mut scope = realm.handle_scope(self.v8_isolate());
+          module_map.lazy_load_es_module_from_code(
+            &mut scope,
+            file_source.specifier,
+            file_source.load()?,
+          )?;
+        }
       }
       self.init_cbs(&realm);
 
diff --git a/core/runtime/snapshot_util.rs b/core/runtime/snapshot_util.rs
index 76dcdaa..ca0ff45 100644
--- a/core/runtime/snapshot_util.rs
+++ b/core/runtime/snapshot_util.rs
@@ -4,6 +4,7 @@ use std::path::Path;
 use std::path::PathBuf;
 use std::time::Instant;
 
+use crate::runtime::jsruntime::BUILTIN_ES_MODULES;
 use crate::runtime::jsruntime::BUILTIN_SOURCES;
 use crate::Extension;
 use crate::ExtensionFileSourceCode;
@@ -57,6 +58,13 @@ pub fn create_snapshot(
       files_loaded_during_snapshot.push(PathBuf::from(path));
     }
   }
+  for source in &BUILTIN_ES_MODULES {
+    if let ExtensionFileSourceCode::LoadedFromFsDuringSnapshot(path) =
+      &source.code
+    {
+      files_loaded_during_snapshot.push(PathBuf::from(path));
+    }
+  }
   for source in js_runtime
     .extensions()
     .iter()


Test patch:
diff --git a/core/modules/tests.rs b/core/modules/tests.rs
index 63a2025..4499e78 100644
--- a/core/modules/tests.rs
+++ b/core/modules/tests.rs
@@ -1380,3 +1380,32 @@ async fn import_meta_resolve_cb() {
   a.await.unwrap().unwrap();
   b.await.unwrap().unwrap();
 }
+
+#[test]
+fn builtin_core_module() {
+  let main_specifier = resolve_url("file:///main_module.js").unwrap();
+
+  let source_code =
+    r#"import { core, primordials, internals } from "ext:core/mod.js";
+if (typeof core === "undefined") throw new Error("core missing");
+if (typeof primordials === "undefined") throw new Error("core missing");
+if (typeof internals === "undefined") throw new Error("core missing");
+"#
+    .to_string();
+  let loader =
+    StaticModuleLoader::new([(main_specifier.clone(), source_code.into())]);
+
+  let mut runtime = JsRuntime::new(RuntimeOptions {
+    module_loader: Some(Rc::new(loader)),
+    ..Default::default()
+  });
+
+  let main_id_fut = runtime
+    .load_main_module(&main_specifier, None)
+    .boxed_local();
+  let main_id = futures::executor::block_on(main_id_fut).unwrap();
+
+  #[allow(clippy::let_underscore_future)]
+  let _ = runtime.mod_evaluate(main_id);
+  futures::executor::block_on(runtime.run_event_loop(false)).unwrap();
+}
diff --git a/core/runtime/tests/misc.rs b/core/runtime/tests/misc.rs
index a828fae..78b1627 100644
--- a/core/runtime/tests/misc.rs
+++ b/core/runtime/tests/misc.rs
@@ -10,6 +10,7 @@ use cooked_waker::IntoWaker;
 use cooked_waker::Wake;
 use cooked_waker::WakeRef;
 use futures::future::poll_fn;
+use rstest::rstest;
 use std::borrow::Cow;
 use std::rc::Rc;
 use std::sync::atomic::AtomicBool;
@@ -187,53 +188,112 @@ async fn test_poll_value() {
   }).await;
 }
 
+#[rstest]
+#[case("script", "Promise.resolve(1 + 2)", Ok(Some(3)))]
+#[case(
+  "script",
+  "Promise.resolve(new Promise(resolve => resolve(2 + 2)))",
+  Ok(Some(4))
+)]
+#[case(
+  "script",
+  "Promise.reject(new Error('fail'))",
+  Err("Uncaught Error: fail")
+)]
+#[case("script", "new Promise(resolve => {})", Ok(None))]
+#[case("call", "async () => 1 + 2", Ok(Some(3)))]
+#[case(
+  "call",
+  "async () => { throw new Error('fail'); }",
+  Err("Uncaught Error: fail")
+)]
+#[case("call", "async () => new Promise(resolve => {})", Ok(None))]
+#[case("call", "() => Promise.resolve(1 + 2)", Ok(Some(3)))]
+#[case(
+  "call",
+  "() => Promise.resolve(new Promise(resolve => resolve(2 + 2)))",
+  Ok(Some(4))
+)]
+#[case(
+  "call",
+  "() => Promise.reject(new Error('fail'))",
+  Err("Uncaught Error: fail")
+)]
+#[case("call", "() => new Promise(resolve => {})", Ok(None))]
+// TODO(mmastrac): This is a bug -- will be fixed
+#[case(
+  "call",
+  "() => { throw new Error('fail'); }",
+  Err("Uncaught undefined")
+)]
+// TODO(mmastrac): This is a bug -- will be fixed
+#[case(
+  "call",
+  "() => { Promise.reject(new Error('fail')); return 1; }",
+  Err("Uncaught (in promise) Error: fail")
+)]
+// TODO(mmastrac): This is a bug -- will be fixed
+#[case(
+  "call",
+  "() => { Deno.core.reportUnhandledException(new Error('fail')); }",
+  Err("Uncaught undefined")
+)]
+// TODO(mmastrac): This is a bug -- will be fixed
+#[case("call", "() => { Deno.core.reportUnhandledException(new Error('fail')); willNotCall(); }", Err("Uncaught undefined"))]
 #[tokio::test]
-async fn test_resolve_value() {
+async fn test_resolve_value(
+  #[case] runner: &'static str,
+  #[case] code: &'static str,
+  #[case] output: Result<Option<u32>, &'static str>,
+) {
+  test_resolve_value_generic(runner, code, output).await
+}
+
+async fn test_resolve_value_generic(
+  runner: &'static str,
+  code: &'static str,
+  output: Result<Option<u32>, &'static str>,
+) {
   let mut runtime = JsRuntime::new(Default::default());
-  let value_global = runtime
-    .execute_script_static("a.js", "Promise.resolve(1 + 2)")
-    .unwrap();
-  let result_global = runtime.resolve_value(value_global).await.unwrap();
-  {
-    let scope = &mut runtime.handle_scope();
-    let value = result_global.open(scope);
-    assert_eq!(value.integer_value(scope).unwrap(), 3);
-  }
+  let result_global = if runner == "script" {
+    let value_global: v8::Global<v8::Value> =
+      runtime.execute_script_static("a.js", code).unwrap();
+    runtime.resolve_value(value_global).await
+  } else if runner == "call" {
+    let value_global = runtime.execute_script_static("a.js", code).unwrap();
+    let function: v8::Global<v8::Function> =
+      unsafe { std::mem::transmute(value_global) };
+    runtime.call_and_await(&function).await
+  } else {
+    unreachable!()
+  };
+  let scope = &mut runtime.handle_scope();
 
-  let value_global = runtime
-    .execute_script_static(
-      "a.js",
-      "Promise.resolve(new Promise(resolve => resolve(2 + 2)))",
-    )
-    .unwrap();
-  let result_global = runtime.resolve_value(value_global).await.unwrap();
-  {
-    let scope = &mut runtime.handle_scope();
-    let value = result_global.open(scope);
-    assert_eq!(value.integer_value(scope).unwrap(), 4);
+  match output {
+    Ok(None) => {
+      let error_string = result_global.unwrap_err().to_string();
+      assert_eq!(
+        "Promise resolution is still pending but the event loop has already resolved.",
+        error_string,
+      );
+    }
+    Ok(Some(v)) => {
+      let value = result_global.unwrap();
+      let value = value.open(scope);
+      assert_eq!(value.integer_value(scope).unwrap(), v as i64);
+    }
+    Err(e) => {
+      let Err(err) = result_global else {
+        let value = result_global.unwrap();
+        let value = value.open(scope);
+        panic!(
+          "Expected an error, got {}",
+          value.to_rust_string_lossy(scope)
+        );
+      };
+      assert_eq!(e, err.downcast::<JsError>().unwrap().exception_message);
+    }
   }
-
-  let value_global = runtime
-    .execute_script_static("a.js", "Promise.reject(new Error('fail'))")
-    .unwrap();
-  let err = runtime.resolve_value(value_global).await.unwrap_err();
-  assert_eq!(
-    "Uncaught Error: fail",
-    err.downcast::<JsError>().unwrap().exception_message
-  );
-
-  let value_global = runtime
-    .execute_script_static("a.js", "new Promise(resolve => {})")
-    .unwrap();
-  let error_string = runtime
-    .resolve_value(value_global)
-    .await
-    .unwrap_err()
-    .to_string();
-  assert_eq!(
-    "Promise resolution is still pending but the event loop has already resolved.",
-    error_string,
-  );
 }
 
 #[test]
diff --git a/core/runtime/tests/snapshot.rs b/core/runtime/tests/snapshot.rs
index fbf9b0d..d8dc46f 100644
--- a/core/runtime/tests/snapshot.rs
+++ b/core/runtime/tests/snapshot.rs
@@ -147,7 +147,8 @@ fn es_snapshot() {
       )
       .unwrap()
     };
-    assert_eq!(i, id);
+    // There's always one internal `deno_core` ES module loaded, so +1 here.
+    assert_eq!(i + 1, id);
 
     #[allow(clippy::let_underscore_future)]
     let _ = runtime.mod_evaluate(id);