Fix #1265 (#1277)

* Fix #1265 * Temorarily disabling parallel execution and junit reporting for worker executor tests to disable flakyness * Fix
golemcloud · Jan 29, 2025 · 2aa5b23 · 2aa5b23
1 parent db7e73f
commit 2aa5b23
Show file tree

Hide file tree

Showing 11 changed files with 117 additions and 28 deletions.
diff --git a/Makefile.toml b/Makefile.toml
@@ -192,59 +192,57 @@ args = [
 description = "Runs worker executor tests only (group 1/8)"
 env = { "RUST_BACKTRACE" = "1", "WASMTIME_BACKTRACE_DETAILS" = "1", "RUST_LOG" = "info", "RUST_TEST_TIME_INTEGRATION" = "5000,30000" }
 script = '''
-cargo test --package golem-worker-executor-base --test integration :tag: -- --nocapture --report-time $JUNIT_OPTS
-cargo test --package golem-worker-executor-base --test integration :tag:group1 -- --nocapture --report-time $JUNIT_OPTS
+cargo test --package golem-worker-executor-base --test integration :tag: -- --report-time
+cargo test --package golem-worker-executor-base --test integration :tag:group1 -- --report-time
 '''
 
 [tasks.worker-executor-tests-group2]
 description = "Runs worker executor tests only (group 2/8)"
 env = { "RUST_BACKTRACE" = "1", "WASMTIME_BACKTRACE_DETAILS" = "1", "RUST_LOG" = "info", "RUST_TEST_TIME_INTEGRATION" = "5000,30000" }
 script = '''
-cargo test --package golem-worker-executor-base --test integration :tag:group2 -- --nocapture --report-time $JUNIT_OPTS
+cargo test --package golem-worker-executor-base --test integration :tag:group2 -- --report-time
 '''
 
 [tasks.worker-executor-tests-group3]
 description = "Runs worker executor tests only (group 3/8)"
 env = { "RUST_BACKTRACE" = "1", "WASMTIME_BACKTRACE_DETAILS" = "1", "RUST_LOG" = "info", "RUST_TEST_TIME_INTEGRATION" = "5000,30000" }
 script = '''
-cargo test --package golem-worker-executor-base --test integration :tag:group3 -- --nocapture --report-time $JUNIT_OPTS
+cargo test --package golem-worker-executor-base --test integration :tag:group3 -- --report-time
 '''
 
 [tasks.worker-executor-tests-group4]
 description = "Runs worker executor tests only (group 4/8)"
 env = { "RUST_BACKTRACE" = "1", "WASMTIME_BACKTRACE_DETAILS" = "1", "RUST_LOG" = "info", "RUST_TEST_TIME_INTEGRATION" = "5000,30000" }
 script = '''
-cargo test --package golem-worker-executor-base --test integration :tag:group4 -- --nocapture --report-time $JUNIT_OPTS --test-threads=1
+cargo test --package golem-worker-executor-base --test integration :tag:group4 -- --report-time $JUNIT_OPTS
 '''
-# NOTE: temporarily set test-threads=1 to debug flakyness
 
 [tasks.worker-executor-tests-group5]
 description = "Runs worker executor tests only (group 5/8)"
 env = { "RUST_BACKTRACE" = "1", "WASMTIME_BACKTRACE_DETAILS" = "1", "RUST_LOG" = "info", "RUST_TEST_TIME_INTEGRATION" = "5000,30000" }
 script = '''
-cargo test --package golem-worker-executor-base --test integration :tag:group5 -- --nocapture --report-time $JUNIT_OPTS
+cargo test --package golem-worker-executor-base --test integration :tag:group5 -- --report-time
 '''
 
-
 [tasks.worker-executor-tests-group6]
 description = "Runs worker executor tests only (group 6/8)"
 env = { "RUST_BACKTRACE" = "1", "WASMTIME_BACKTRACE_DETAILS" = "1", "RUST_LOG" = "info", "RUST_TEST_TIME_INTEGRATION" = "5000,30000" }
 script = '''
-cargo test --package golem-worker-executor-base --test integration :tag:group6 -- --nocapture --report-time $JUNIT_OPTS
+cargo test --package golem-worker-executor-base --test integration :tag:group6 -- --report-time
 '''
 
 [tasks.worker-executor-tests-group7]
 description = "Runs worker executor tests only (group 7/8)"
 env = { "RUST_BACKTRACE" = "1", "WASMTIME_BACKTRACE_DETAILS" = "1", "RUST_LOG" = "info", "RUST_TEST_TIME_INTEGRATION" = "5000,30000" }
 script = '''
-cargo test --package golem-worker-executor-base --test integration :tag:group7 -- --nocapture --report-time $JUNIT_OPTS
+cargo test --package golem-worker-executor-base --test integration :tag:group7 -- --report-time
 '''
 
 [tasks.worker-executor-tests-group8]
 description = "Runs worker executor tests only (group 8/8)"
 env = { "RUST_BACKTRACE" = "1", "WASMTIME_BACKTRACE_DETAILS" = "1", "RUST_LOG" = "info", "RUST_TEST_TIME_INTEGRATION" = "5000,30000" }
 script = '''
-cargo test --package golem-worker-executor-base --test integration :tag:group8 -- --nocapture --report-time $JUNIT_OPTS
+cargo test --package golem-worker-executor-base --test integration :tag:group8 -- --report-time
 '''
 
 [tasks.integration-tests]

diff --git a/golem-worker-executor-base/tests/rust_rpc_stubless.rs b/golem-worker-executor-base/tests/rust_rpc_stubless.rs
@@ -1143,3 +1143,65 @@ async fn ephemeral_worker_invocation_via_rpc1(
         _ => panic!("Unexpected result value"),
     }
 }
+
+#[test]
+#[tracing::instrument]
+async fn golem_bug_1265_test(
+    last_unique_id: &LastUniqueId,
+    deps: &WorkerExecutorTestDependencies,
+    _tracing: &Tracing,
+) {
+    let context = TestContext::new(last_unique_id);
+    let executor = start(deps, &context).await.unwrap();
+
+    let counters_component_id = executor.store_component("counters").await;
+    let caller_component_id = executor
+        .store_component_with_dynamic_linking(
+            "caller",
+            &[
+                (
+                    "rpc:counters-client/counters-client",
+                    DynamicLinkedInstance::WasmRpc(DynamicLinkedWasmRpc {
+                        target_interface_name: HashMap::from_iter(vec![
+                            ("api".to_string(), "rpc:counters-exports/api".to_string()),
+                            (
+                                "counter".to_string(),
+                                "rpc:counters-exports/api".to_string(),
+                            ),
+                        ]),
+                    }),
+                ),
+                (
+                    "rpc:ephemeral-client/ephemeral-client",
+                    DynamicLinkedInstance::WasmRpc(DynamicLinkedWasmRpc {
+                        target_interface_name: HashMap::from_iter(vec![(
+                            "api".to_string(),
+                            "rpc:ephemeral-exports/api".to_string(),
+                        )]),
+                    }),
+                ),
+            ],
+        )
+        .await;
+
+    let mut env = HashMap::new();
+    env.insert(
+        "COUNTERS_COMPONENT_ID".to_string(),
+        counters_component_id.to_string(),
+    );
+    let caller_worker_id = executor
+        .start_worker_with(&caller_component_id, "rpc-counters-bug1265", vec![], env)
+        .await;
+
+    let result = executor
+        .invoke_and_await(
+            &caller_worker_id,
+            "rpc:caller-exports/caller-inline-functions.{bug-golem1265}",
+            vec![Value::String("test".to_string())],
+        )
+        .await;
+
+    drop(executor);
+
+    check!(result == Ok(vec![Value::Result(Ok(None))]));
+}
diff --git a/test-components/caller.wasm b/test-components/caller.wasm
diff --git a/test-components/caller_composed.wasm b/test-components/caller_composed.wasm
diff --git a/test-components/counters.wasm b/test-components/counters.wasm
diff --git a/test-components/ephemeral.wasm b/test-components/ephemeral.wasm
diff --git a/test-components/rpc/caller/src/lib.rs b/test-components/rpc/caller/src/lib.rs
@@ -116,6 +116,17 @@ impl Guest for Component {
 
         vec![(name1, key1), (name2, key2), (name3, key3)]
     }
+
+    fn bug_golem1265(s: String) -> Result<(), String> {
+        println!("Reproducer for golem issue #1265");
+        let component_id =
+            env::var("COUNTERS_COMPONENT_ID").expect("COUNTERS_COMPONENT_ID not set");
+        let counters_uri = Uri {
+            value: format!("urn:worker:{component_id}/bug1265"),
+        };
+        let api = Api::new(&counters_uri);
+        api.blocking_bug_golem1265(&s)
+    }
 }
 
 fn create_use_and_drop_counters(counters_uri: &Uri) {

diff --git a/test-components/rpc/caller/wit/caller.wit b/test-components/rpc/caller/wit/caller.wit
@@ -12,4 +12,6 @@ world caller {
   export bug-wasm-rpc-i32: func(in: timeline-node) -> timeline-node;
 
   export ephemeral-test1: func() -> list<tuple<string, string>>;
+
+  export bug-golem1265: func(s: string) -> result<_, string>;
 }
diff --git a/test-components/rpc/counters/src/lib.rs b/test-components/rpc/counters/src/lib.rs
@@ -42,6 +42,11 @@ impl Guest for Component {
     fn bug_wasm_rpc_i32(in_: TimelineNode) -> TimelineNode {
         in_
     }
+
+    fn bug_golem1265(s: String) -> Result<(), String> {
+        eprintln!("Got {s}");
+        Ok(())
+    }
 }
 
 pub struct Counter {

diff --git a/test-components/rpc/counters/wit/counters.wit b/test-components/rpc/counters/wit/counters.wit
@@ -20,6 +20,8 @@ interface api {
   }
 
   bug-wasm-rpc-i32: func(in: timeline-node) -> timeline-node;
+
+  bug-golem1265: func(s: string) -> result<_, string>;
 }
 
 world counters {

diff --git a/wasm-rpc/src/wasmtime.rs b/wasm-rpc/src/wasmtime.rs
@@ -348,14 +348,21 @@ async fn decode_param_impl(
         Type::Result(ty) => match param {
             Value::Result(result) => match result {
                 Ok(value) => {
-                    let ok_ty = ty.ok().ok_or(EncodingError::ValueMismatch {
-                        details: format!("in {context} could not get ok type"),
-                    })?;
                     let decoded_value = match value {
-                        Some(v) => Some(
-                            decode_param_impl(v, &ok_ty, resource_store, &format!("{context}.ok"))
+                        Some(v) => {
+                            let ok_ty = ty.ok().ok_or(EncodingError::ValueMismatch {
+                                details: format!("in {context} could not get ok type"),
+                            })?;
+                            Some(
+                                decode_param_impl(
+                                    v,
+                                    &ok_ty,
+                                    resource_store,
+                                    &format!("{context}.ok"),
+                                )
                                 .await?,
-                        ),
+                            )
+                        }
                         None => None,
                     };
                     match decoded_value {
@@ -367,19 +374,21 @@ async fn decode_param_impl(
                     }
                 }
                 Err(value) => {
-                    let err_ty = ty.err().ok_or(EncodingError::ValueMismatch {
-                        details: format!("in {context} could not get err type"),
-                    })?;
                     let decoded_value = match value {
-                        Some(v) => Some(
-                            decode_param_impl(
-                                v,
-                                &err_ty,
-                                resource_store,
-                                &format!("{context}.err"),
+                        Some(v) => {
+                            let err_ty = ty.err().ok_or(EncodingError::ValueMismatch {
+                                details: format!("in {context} could not get err type"),
+                            })?;
+                            Some(
+                                decode_param_impl(
+                                    v,
+                                    &err_ty,
+                                    resource_store,
+                                    &format!("{context}.err"),
+                                )
+                                .await?,
                             )
-                            .await?,
-                        ),
+                        }
                         None => None,
                     };
-Original file line number
+Diff line change
@@ Expand Up / @@ -20,6 +20,8 @@ interface api { @@
       }
       bug-wasm-rpc-i32: func(in: timeline-node) -> timeline-node;
+      bug-golem1265: func(s: string) -> result<_, string>;
     }
     world counters {
@@ Expand Down @@