jasmine / jasmine-npm

A jasmine runner for node projects.
MIT License
378 stars 145 forks source link

Jasmine in parallel mode cannot report bigint comparison errors #212

Open Darker opened 1 week ago

Darker commented 1 week ago

Steps to Reproduce

  1. Create a test in which a BigInt comparison fails
  2. Run the test with --parallel
  3. Despair, because you don't know why is the test failing

Expected Behavior

Failures:
1) bigIntFunctions_test big int error test
  Message:
    Expected 2 to be 1.
  Stack:
        at <Jasmine>
        at UserContext.<anonymous> (bigIntFunctions_test.js:61:33)
        at <Jasmine>

Actual Behavior

Suite error: bigIntFunctions_test
  Message:
    TypeError: Do not know how to serialize a BigInt
  Stack:
        at stringify (<anonymous>)
        at writeChannelMessage (node:internal/child_process/serialization:159:20)
        at target._send (node:internal/child_process:845:17)
        at target.send (node:internal/child_process:745:19)
        at Worker.send (node:internal/cluster/worker:48:10)
        at reporter.<computed> [as specDone] (node_modules/jasmine/lib/parallel_worker.js:163:21)
        at <Jasmine>

Example code that reproduces the problem

Write a test like this:

describe("bigIntFunctions_test", ()=>{
    it("big int error test", ()=>{
        function returnsBigInt() {
            return 2n;
        }
        function returnsDifferentBigInt() {
            return 1n;
        }
        expect(returnsBigInt()).toBe(returnsDifferentBigInt());
    })
});

Run with:

jasmine --config=tests/jasmine_unit.json --parallel=14

Using a config like this:

{
    "spec_dir": "tests",
    "spec_files": [
      "*_test.js"
    ],
    "helpers": [

    ],
    "stopSpecOnExpectationFailure": false,
    "random": true
  }


### Possible Solution

I am not sure if this should be handled by Node.JS team, seems like a blind spot in their serialization. If the values are only needed for reporting I'd just convert them to string as a workaround now.

### Context

This prevents running these tests in parallel mode.

### jasmine-core version

jasmine-core v5.1.2

### Versions of other relevant packages

jasmine v5.1.0
jasmine-core v5.1.2

### Node.js or browser version

v20.2.0

### Operating System

Ubuntu 20 - does it matter really?
sgravrock commented 1 week ago

This could be the tip of a medium sized iceberg, since bigint isn't the only kind of value that isn't JSON serializable.

https://github.com/GoogleChromeLabs/jsbi/issues/30#issuecomment-521449285 is relevant.

I wouldn't expect Node to take care of this kind of thing. They theoretically could, since they control both ends of the pipe, but it's much simpler to just say that cluster communication uses JSON. So it's probably up to Jasmine to make this work. We could JSON-serialize our own messages and convert bigints to something like {"__jasmine_special_type": "BigInt", "value": "<string representation of the bigint>"} and then reverse the conversion on the other side.

On the other hand, serialization and deserialization in JS is inherently lossy. bigint is one of the easy cases. There are plenty of objects that can't be serialized and deserialized without losing information. It might be better to set the expectation that the expected and actual properties of Expectation are a best-effort approximation in parallel mode, and just convert bigints to strings.