Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jasmine in parallel mode cannot report bigint comparison errors #212

Closed
Darker opened this issue Sep 10, 2024 · 2 comments
Closed

Jasmine in parallel mode cannot report bigint comparison errors #212

Darker opened this issue Sep 10, 2024 · 2 comments
Labels

Comments

@Darker
Copy link

Darker commented Sep 10, 2024

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
Copy link
Member

sgravrock commented Sep 12, 2024

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

GoogleChromeLabs/jsbi#30 (comment) 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.

@sgravrock sgravrock transferred this issue from jasmine/jasmine Sep 12, 2024
@sgravrock sgravrock added the bug label Sep 12, 2024
@sgravrock
Copy link
Member

Actually, Jasmine could probably just report a placeholder like '<not serializable>' whenever the expected or actual property of a passed or failed expectation fails to serialize. That would be simpler, and the odds are that nobody would ever know the difference. Those properties are pretty obscure. They don't even make sense for all expectations, and I've never seen or heard of a reporter that actually uses them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants