jest: [Bug]: Watch fails if SL (Steam Locomotive) is installed

I would like to suggest that fixing this issue should include displaying a more descriptive error in this case than the one it does:

  ● Test suite failed to run

thrown: [Error]

gosh I feel super frustrated when the worlds collide and you can’t understand why that’s happening.

the error message desperately need fixing. luckily, we do know the culprit in this example, but next time it could be some other piece of software causing interference and as-is there’s not a single clue to tell the reason and debug.

This is the obviously correct solution.

Unlinking/uninstalling sl is a correct temporary workaround for this issue. (You could also use direnv to selectively alias it to false when working on jest projects, or wrap it in something that detects if jest is calling it, maybe…)

Homebrew has > 20k people who have installed sl in the past year, debian popcon has > 1% of submitted reports with an install. I think that is more than enough usage to not declare incompatibility. It’d make me sad, too – there is far too little whimsy on my computer and I do not like the idea that since one tool I must use has mistaken assumptions, I should have to reduce that further.

but what should that error say?

I think that it should pass through the existing error, something like 'Command failed with ENAMETOOLONG: sl status -amnu /Users/nokel81/repos/lens/packages/core\n' + 'spawn ENAMETOOLONG',

This case doesn’t need a special error, because the underlying issue should be fixed by one of the several proposals (a sapling command, only running sl if the scm configuration option is set to sapling, etc).

Just having that error show up would unblock people, because you can see “something is up with sl”.

I’m asking for this also because this is not the only Jest error I’ve seen that failed with thrown: [Error] (the last ended up being something with watchman) and I’m hoping to get some insight into the next one of those I see, as well.

I just ran into the same issue; this needs to be fixed. I just spent a bunch of time debugging core jest code to properly have this error be thrown.

@rmartine-ias I fully agree that uninstalling is a good temporary workaround. What do you think a more permanent solution to the jest --watch incompatibility with everyone’s favorite steam locomotive ❤️?

A config option? or something better possibly?

I am not familiar with the project’s internals, or what tradeoffs the maintainers make, so I don’t think I am the person to ask. I am probably missing some context or misreading some code.

That said, I would:

• Fix the error message system
• Only find roots once, when starting the watch process
• Store SCM source at that time on a new Root object
• Find sl roots through directory traversal, instead of binary calling (note: not sure if you’d need to look for sapling-specific files because of the git compatibility)
• AND/OR: Only find sl roots if any of the sapling configuration files exist

I think this would get:

• No errors, because sl isn’t called unless a sapling root is found, and/or a sapling config file exists
• No need for additional configuration options, which would break existing workflows when mercurial/sapling users have to add the new option across all machines (including CI)

It would make it so that adding or removing SCM roots while running watch does not work as prior, though. I am sure somebody is doing this. The “only find roots once” thing can be dropped; that is a performance enhancement of questionable value.

I’m liking the suggestion here to ship a sapling command that could be used in this scenario instead of sl. It’s certainly not the most immediate solution because it would require Sapling users to upgrade to a version that includes it (whenever that happens), but it does feel like the most robust suggestion I’ve seen.

I think it would also avoid the issue of having to add some sort of scm option to Jest, and I think it also avoids the issue with any error message weirdness.

Of course, this all assumes there isn’t already some other tool that provides a sapling command. 😅

@SimenB I called in the pros

A framework whose purpose is to support quality approaches, with an error message like this (“thrown: [Error]”), with no fix in the 10 months since the bug was reported - it is shameful. I just lost hours to this.

By merely providing a clearer error message this would be greatly remediated, at least as a stopgap measure until a more sophisticated improvement can be made.

Why not use $(npm bin)/sl to call sapling? Ah, because it’s installed using brew as well, sorry.

In that case: Why not use $(brew --prefix sapling)/bin/sl to call sapling?

Not stale, I like the idea of a config that defaults to git

I uninstalled sl as my solution

This is the obviously correct solution. The problem is that the error message is so bad people don’t realize they need to do this. I think we can check the size of the sl binary to provide a better error message.

@sTRAGER also had another idea to improve the error reporting, but needs to elaborate it more

I think that would work. If you run sl --version with SL installed, you’ll get the steam locomotive yet again. 😄

@rmartine-ias https://github.com/jestjs/jest/issues/14046#issuecomment-1555050052

or wrap it in something that detects if jest is calling it, maybe…)

I created a workaround using that approach, since jest checks for sapling with sl root

#!/usr/bin/env zsh

if [[ $1 == 'root' ]]; then
  exit 1
fi

exec /opt/homebrew/bin/sl "$@"

This could be patched into the sl binary either upstream or just in homebrew and other distributions’ builds.

👍

@rmartine-ias I agree with you. If I would have seen that error I would have been able to find this bug report much faster. I only found it because I also started digging in node_modules for answers. Showing the error would help more people finding a faster solution.

I uninstalled sl as my solution, I had even forgotten that I installed it just for fun ages ago.

We might need to revert the sapling support and figure out a better approach. The timeout suggested in https://github.com/facebook/jest/pull/14061 seems icky to me. My own suggestion of grepping man pages or crawling the FS doesn’t seem good either.

/cc @vegerot

Copying from my own issue (which is now closed):

Having steam locomotive installed (on macOS via brew) caused watch runs to fail with a weird error:

  ● Test suite failed to run

thrown: [Error]

after digging around it turned out that the formatting is bad (and probably should also be fixed) but that opaque error is actually the following:

Error
    at ChildProcess.spawn (node:internal/child_process:413:11)
    at Object.spawn (node:child_process:700:9)
    at execa (/Users/nokel81/repos/lens/node_modules/jest-changed-files/node_modules/execa/index.js:83:26)
    at Object.findChangedFiles (/Users/nokel81/repos/lens/node_modules/jest-changed-files/build/sl.js:104:43)
    at /Users/nokel81/repos/lens/node_modules/jest-changed-files/build/index.js:51:17
    at Array.map (<anonymous>)
    at getChangedFilesForRoots (/Users/nokel81/repos/lens/node_modules/jest-changed-files/build/index.js:50:43)
    at processTicksAndRejections (node:internal/process/task_queues:96:5) {
  errno: -63,
  code: 'ENAMETOOLONG',
  syscall: 'spawn',
  originalMessage: 'spawn ENAMETOOLONG',
  shortMessage: 'Command failed with ENAMETOOLONG: sl status -amnu /Users/nokel81/repos/lens/packages/core\n' +
    'spawn ENAMETOOLONG',
  command: 'sl status -amnu /Users/nokel81/repos/lens/packages/core',
  escapedCommand: 'sl status -amnu "/Users/nokel81/repos/lens/packages/core"',
  exitCode: undefined,
  signal: undefined,
  signalDescription: undefined,
  stdout: '',
  stderr: '',
  all: '',
  failed: true,
  timedOut: false,
  isCanceled: false,
  killed: false
}