jest icon indicating copy to clipboard operation
jest copied to clipboard

[Bug]: False negative (instanceof Float32Array)

Open o-alexandre-felipe opened this issue 3 years ago • 3 comments

Version

27.1.1

Steps to reproduce

I prepared an example here, simply run npm install then you have the node run.js that shows the expected behavior (without jest), and npx jest will show the behavior when running in jest.

Expected behavior

In this instance I expected that Float32Array created by native code to be instanceof Float32Array.

Actual behavior

It seems that jest redefined Float32Array globally, maybe we could simply surround commands like this by some function that would restore the native functions. jest.withNativeTypes( () => do my test )

Additional context

onnxjs is a frame work to evaluate neural network models in the format ONNX, it supports different backends. onnxjs-node enables the use of onnxruntime in onnxjs.

These libraries use tensors objects, a tensor class is a multidimensional view of a typed array. The outputs of a neural network are constructed in NAPI as typed arrays [1].

Both the native code and javascript code have typed array maps [2],[3], and the javascript code checks if the value returned is an array of the expected type[4]. Normally this works, but if we invoke this from a jest test, that check will evaluate to false, and finally it will throw an error[5].

Environment

For a way to reproduce, check Dockerfile, and docker-compose.yaml.

o-alexandre-felipe avatar Sep 10 '21 11:09 o-alexandre-felipe

I've come across this problem too. It appears to be related to/a recurrence of: https://github.com/facebook/jest/issues/10786, which also replicates for me in version 27.3.1.

As far as I can tell, there is no workaround for this, and Jest cannot be used to test any code using onnxruntime. I'd live with any backdoor way to get access to the original Float32Array, but without the original, no compatible data can be passed. It might be a corner case, but it's a blocking corner case.

morungos avatar Nov 19 '21 00:11 morungos

Hello,

I had this same problem, and after a lot of tinkering I eventually found a workaround.

As far as I can tell, the problem happens because Jest uses the module node:vm to run tests. This module allows running JavaScript code in a different V8 context. The module onnxruntime doesn't recognize the instances of Float32Array because they were defined in another VM context. Their constructor is different from the Float32Array of the current context, so the operator instanceof returns false and an exception is thrown.

For some reason, the utility functions in the module node:utils/types seems immune to this problem : they return the correct result no matter which VM context the argument comes from.

I ended up making use of Symbol.hasInstance to overload the instanceof operator. It's not a clean solution, but it works, and it allows me to run my model using Jest.

// Import Node typing utilities
import * as types from "node:util/types";

// Import onnxruntime-node's default backend
import { onnxruntimeBackend } from "onnxruntime-node/dist/backend";
import { registerBackend } from "onnxruntime-common";

// Define the constructors to monkey-patch
const TYPED_ARRAYS_CONSTRUCTOR_NAMES = [
  "Int8Array",
  "Int16Array",
  "Int32Array",
  "Uint8Array",
  "Uint8ClampedArray",
  "Uint16Array",
  "Uint32Array",
  "Float32Array",
  "Float64Array",
] as const;

// Keep a reference to the original initialization method
const originalMethod = onnxruntimeBackend.init;

// Monkey-patch the initialization function
onnxruntimeBackend.init = function (...args) {
  // There is probably a better way to do this
  Array.isArray = (x: any): x is any[] =>
    typeof x === "object" &&
    x !== null &&
    typeof x.length === "number" &&
    x?.constructor.toString() === Array.toString();

  // For each typed array constructor
  for (const ctorName of TYPED_ARRAYS_CONSTRUCTOR_NAMES) {
    // Get the constructor from the current context
    const ctor: Function = global[ctorName]!;

    // Get the corresponding test function from the `util` module
    const value = types[`is${ctorName}`].bind(types);

    // Monkey-patch the constructor so "x instanceof ctor" returns "types[`is${ctorName}`](x)"
    Object.defineProperty(ctor, Symbol.hasInstance, {
      value,
      writable: false,
      configurable: false,
      enumerable: false,
    });
  }

  // Call the original method
  return originalMethod.apply(this, args);
};

// Register the backend with the highest priority, so it is used instead of the default one
registerBackend("test", onnxruntimeBackend, Number.POSITIVE_INFINITY);

I hope this code can be useful to other people who have the same problem.

m-r-r avatar Sep 28 '22 21:09 m-r-r

Anything new on this issue? The work around by @m-r-r works nicely, but this can't be a viable long-term solution.

mathiasbockwoldt avatar Jan 17 '23 11:01 mathiasbockwoldt

Also just bumped into this issue. Would be nice to have a proper fix for it.

xenova avatar May 02 '23 01:05 xenova

I hit this issue as well. I couldn't get the above workaround to work, so I solved it by mocking Array.isArray. Details are here in case it helps anyone.

adam-harwood avatar Jul 08 '23 01:07 adam-harwood

I don't know whether there should be a general fix in testjs, or onnxruntime should submit a fix for compatibility for supporting testjs.

Is the tfjs PR mentioned above (https://github.com/tensorflow/tfjs/pull/7181) a good example for enhance test compatibility? Perhaps I can port that change to onnxruntime

fs-eire avatar Jul 09 '23 03:07 fs-eire

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

github-actions[bot] avatar Jul 08 '24 04:07 github-actions[bot]

This issue was closed because it has been stalled for 30 days with no activity. Please open a new issue if the issue is still relevant, linking to this one.

github-actions[bot] avatar Aug 07 '24 04:08 github-actions[bot]

FWIW, this is essentially a duplicate of #2549, like https://github.com/jestjs/jest/issues/11864#issuecomment-1261468011 touches upon

SimenB avatar Aug 07 '24 07:08 SimenB