Skip to content

Timers mock panic with sub-test #55849

Description

@axetroy

Version

v22.10.0

Platform

No response

Subsystem

No response

What steps will reproduce the bug?

1. Create a test file a.test.mjs

import test from 'node:test'

test.beforeEach((t) => {
    t.mock.timers.enable({ apis: ['Date'] })
    t.mock.timers.tick(1731488301220) // 2024-11-13 16:58:21.220
})

test('test 1', async (t) => {
    await t.test('sub test 1', async (t) => {
        //
    })
})

2. Run the following command to test

node --test --experimental-default-type=module ./a.test.mjs

How often does it reproduce? Is there a required condition?

Every time

What is the expected behavior? Why is that the expected behavior?

There should be not panic

What do you see instead?

▶ test 1
  ✖ sub test 1 (1.0925ms)
    TypeError [Error]: Cannot redefine property: Symbol(MockTimers)
        at defineProperties (<anonymous>)
        at #createDate (node:internal/test_runner/mock/mock_timers:381:5)
        at Object.Date (node:internal/test_runner/mock/mock_timers:614:45)
        at node:internal/test_runner/mock/mock_timers:635:74
        at Array.forEach (<anonymous>)
        at #toggleEnableTimers (node:internal/test_runner/mock/mock_timers:635:5)
        at MockTimers.enable (node:internal/test_runner/mock/mock_timers:720:29)
        at TestContext.<anonymous> (file:///path/to/a.test.mjs:4:19)
        at TestHook.runInAsyncScope (node:async_hooks:211:14)
        at TestHook.run (node:internal/test_runner/test:934:25)

Additional information

No response

Activity

  1. added
    test_runnerIssues and PRs related to the test runner subsystem.
    confirmed-bugIssues and PRs for confirmed bugs.
    on Nov 14, 2024
  2. avivkeller commented on Nov 14, 2024

    @avivkeller
    Member

    Even simpler reproduction:

    import { test, beforeEach } from 'node:test'
    
    beforeEach((t) => t.mock.timers.enable());
    
    test(() => test());
    $ node repro.js
    

    It's failing here:

    ObjectDefineProperties(MockDate, {
    __proto__: null,
    [kMock]: {
    __proto__: null,
    enumerable: false,
    configurable: false,
    writable: false,
    value: this,
    },
    isMock: {
    __proto__: null,
    enumerable: true,
    configurable: false,
    writable: false,
    value: true,
    },
    });

  3. deokjinkim commented on Nov 14, 2024

    @deokjinkim
    Contributor

    With above example, #createDate is called 2 times. So when I change configurable of [kMock] to true, above example is working. But I'm not sure that this approach is right.

  4. avivkeller commented on Nov 14, 2024

    @avivkeller
    Member

    I don't think we should set it to true.

    IMO The appropriate response should be an error that the mock is already enabled, as is done if you try to enable it twice.

  5. avivkeller commented on Nov 14, 2024

    @avivkeller
    Member

    A better way to look at this is:

    import { test } from 'node:test'
    
    test((outer) => {
        outer.mock.timers.enable()
        test((inner) => inner.mock.timers.enable())
    });

    When outer changes the Date object, it sets these properties.

    Then, when inner wants to change the Date object, it copies those properties, before trying to set them:

    ObjectDefineProperties(
    MockDate,
    dateProps,
    );

    IMO there are two paths that can be taken here:
    A) Throw an error that the object has already been mocked
    B) Exclude those properties, and overwrite the mock.

    I'm inclined for (A).

  6. avivkeller commented on Nov 14, 2024

    @avivkeller
    Member

    Opened #55858

  7. added a commit that references this issue on Nov 17, 2024
    775a100
  8. added a commit that references this issue on Nov 18, 2024
    034505e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    confirmed-bugIssues and PRs for confirmed bugs.test_runnerIssues and PRs related to the test runner subsystem.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions