Detect infinite loops and stop tests

[SimenB]

🚀 Feature Proposal

Similar to Rome.

https://twitter.com/sebmck/status/1114672543133097984. https://github.com/facebookexperimental/rome/blob/1add11d62d787ff646dcc52e8b35cb41c730a30e/packages/%40romejs/core/master/testing/TestRunner.ts#L342-L405

Motivation

Infinite loops can be hard to track down, we should try to help the user. We don't currently attach an inspector, but it shouldn't be too much work doing so

[ayshiff]

I would love to work on it ! 🚀

[ayshiff]

I started working on the feature by analysing Inspector API usage and how Rome is using it.

I made a schema to represent the process, to know if I was going in the right direction. One of my questions concerns how we will make the monitorHeartbeat which pings the workers to check if they are still alive.

[SimenB]

Thanks for picking this up @ayshiff! monitorHeartbeat seems like it'd just me a message passed to the worker, when then responds with it's own message, sort of like ping -> <- pong. Another idea would be to make workers automatically emit a heartbeat e.g. every second, and the parent process could detect when there has been no message for n seconds (another PARENT_MESSAGE)

[ayshiff]

Another idea could be to keep track of loops with more than N iterations as @gaearon did here. Maybe we could try a mix of both ideas ? I don't know what is the best way to go !

[SimenB]

That could work, but I think ideally we'd want something with the inspector, I don't think we want to rely on some babel plugin to run

[gaearon]

Another downside of Babel plugin is you have to hardcode a number of iterations.

[ayshiff]

I've been stuck in my progress for a moment.

This is where I am:

• Initialize the inspector and connect a session to it. Then we can enable the Debugger.
• Inside jest-worker I created a new message PARENT_MESSAGE_HEARTBEAT that is emitted each time the child process has work to do. When there has been no message received from the child for N time, we consider that the worker is dead and we call the Debugger.pause and wait for the Debugger.paused response. Once we get the response, we can retrieve the stack trace.

The monitorHeartbeat is not working properly at the moment. In fact, I was wondering if you could give some explanations about the way the jest-worker package is working. I don't want to have misunderstandings on important parts of the code.

I will need some help on this one 👍

[jeysal]

Hi @ayshiff and sorry this went under the radar for months. It sounds like you already got quite far and it'd be a shame to lose all this work. I was not involved in writing jest-worker (neither child processes nor worker threads) but I landed a major refactor of the way the workers terminate last year, so I can possibly help / we can figure things out together. I didn't catch any specifics from your comment yet, but if you're still interested, let me know what problem you ran into :)

[ayshiff]

Hi @jeysal ! No problem, it's been a long time since I looked at my code. I'll take a look at what I did and I'll make a Draft PR with some explanations of what I tried to do (and surely some questions) which will be much more simple for you to review 👍

[octogonz]

The Node.js runInContext() API is used by Jest to run scripts, and its options include a timeout. Maybe that could be used, at least to ensure that the test runner halts and reports an error, instead of your CI job hanging indefinitely?

[SimenB]

@octogonz happy to take a PR adding some sort of sane timeout (15 minutes? 30? 60? people might have looong running e2e tests or something)

[ratikkoka]

Would a CLI option be appropriate for something like this? Such as --timeout=15 If so, I would love to implement it. It would be helpful to have more insight as to the implementation of CLI options (such as where to find the implementation of --bail or --testTimeout). Thanks!

[ratikkoka]

@SimenB please let me know if this work and whether implementing a --timeout is something that would be a good thing to work on!

[covik]

I'm working on a shortPoll implementation using recursive functions and async code. The code executes async action (function passed as argument), waits for certain amount of milliseconds and repeats the process indefinitely. While validating my tests (kind of manual mutation testing) i swapped the order of sleep() and run() to see if test fails. The test went into infinite loop and never failed so I figured I could give you full code if you need it for your own tests? I know it's easy to create infinite loop but this is more like an infinite recursion (which never stack overflows, for some reason).

I'm using jest 26:

import { describe, expect, it, jest } from '@jest/globals'
import fakeTimers from '@sinonjs/fake-timers'

type ActionHandler<T = unknown> = () => Promise<T>

const sleep = {
  now: (ms: number) => new Promise(resolve => setTimeout(resolve, ms))
}

const shortPoll = {
  do: function <T = unknown> (
    action: ActionHandler<T>,
    delayInMilliseconds: number
  ) {
    const run = async () => {
      await action()
      await sleep.now(delayInMilliseconds)
      await run()
    }

    void run()
  }
}

describe('shortPoll', () => {
  it('should execute action, wait delayInMilliseconds and repeat indefinitely', async () => {
    const clock = fakeTimers.install()
    const delay = 2000
    const action = jest.fn()

    shortPoll.do(action as ActionHandler, delay)

    // by advancing the timer five times assume the function runs indefinitely
    // also validate action() was called immediately without any timers when shortPoll was first called
    // if you move sleep() before action() this test will report less invocation calls than expected
    // if you move run() before sleep() the tests goes into infinite loop and never fails
    const initialInvocation = 1
    const totalCycles = 5
    await clock.tickAsync(delay * totalCycles)
    expect(action).toHaveBeenCalledTimes(initialInvocation + totalCycles)

    clock.uninstall()
  })
})

[github-actions[bot]]

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]]

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]]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.