nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
108.11k stars 29.86k forks source link

Flaky js-native-api/test_cannot_run_js/test #48180

Closed kvakil closed 9 months ago

kvakil commented 1 year ago

Test

js-native-api/test_cannot_run_js/test

Platform

Linux ARM64, Linux x64, macOS ARM64, macOS x64

Console output

crashed (-6)
---
duration_ms: 509.947
exitcode: -6
severity: crashed
...

Build links

See reliability report: https://github.com/nodejs/reliability/issues/576

Here's one example: https://ci.nodejs.org/job/node-test-binary-armv7l/5897/RUN_SUBSET=native,nodes=ubuntu2004-armv7l/testReport/junit/js-native-api/test_cannot_run_js/test/

Additional information

Newly added in https://github.com/nodejs/node/pull/47986

Probably comes from this abort statement: https://github.com/nodejs/node/blob/847b9e08849f391ac3c21a73fd375e86b97605b8/test/js-native-api/test_cannot_run_js/test_cannot_run_js.c#L18

Could not reproduce locally with test.py. Weirdly it reproduces every time when run without test.py:

./node test/js-native-api/test_cannot_run_js/test.js 
abmusse commented 9 months ago

I think this issue is showing up again I've seen it on the following platforms:

AIX: https://ci.nodejs.org/view/All/job/node-test-commit-aix/50418/nodes=aix72-ppc64/testReport/js-native-api/test_cannot_run_js/test_/ IBM i : https://ci.nodejs.org/job/node-test-commit-ibmi/nodes=ibmi73-ppc64/1466/testReport/js-native-api/test_cannot_run_js/test_/

richardlau commented 9 months ago

This issue should not have been closed -- https://github.com/nodejs/node/pull/48181 merely marked the test as flaky.

abmusse commented 9 months ago

I've added a log to check what the actual and expected values are before we abort.

This was done on IBM i:

https://github.com/nodejs/node/blob/847b9e08849f391ac3c21a73fd375e86b97605b8/test/js-native-api/test_cannot_run_js/test_cannot_run_js.c#L15-L19

diff --git a/test/js-native-api/test_cannot_run_js/test_cannot_run_js.c b/test/js-native-api/test_cannot_run_js/test_cannot_run_js.c
index c495f8780d..3ed3ee97fb 100644
--- a/test/js-native-api/test_cannot_run_js/test_cannot_run_js.c
+++ b/test/js-native-api/test_cannot_run_js/test_cannot_run_js.c
@@ -14,9 +14,12 @@ static void Finalize(napi_env env, void* data, void* hint) {

   if (napi_delete_reference(env, *ref) != napi_ok) abort();
   if (napi_get_global(env, &global) != napi_ok) abort();
-  if (napi_get_named_property(env, global, "setTimeout", &set_timeout) !=
-      expected_status)
+
+  napi_status actual_status = napi_get_named_property(env, global, "setTimeout", &set_timeout);
+  if (actual_status != expected_status){
+    fprintf(stderr, "actual_status (%d) != expected_status(%d)\n", actual_status, expected_status);
     abort();
+  }
   free(ref);
 }
$ ./out/Release/node test/js-native-api/test_cannot_run_js/test.js 
actual_status (0) != expected_status(10)
IOT/Abort trap (core dumped)

Looks like the actual napi_status value is 0 but the expected status is 10. 🤔

From the docs napi_status is a enum an 0 should map to -> napi_ok and 10 would map to -> napi_pending_exception

ref: https://nodejs.org/api/n-api.html#napi_status