search

signalapp / libsignal

Home to the Signal Protocol as well as other cryptographic primitives which make Signal possible.
GNU Affero General Public License v3.0
3.62k stars 420 forks source link

yarn test fails with segmentation fault #459

Closed Niteo closed 2 years ago

Niteo commented 2 years ago

I've been following the steps to build libsignal for node:

$ cd node
$ nvm use
$ yarn install
$ yarn tsc
$ yarn test

Everything seems to build without errors (ends with gyp info ok), but when running yarn test I get a segmentation fault:

yarn run v1.21.1
$ electron-mocha --recursive dist/test --require source-map-support/register
Segmentation fault (core dumped)
error Command failed with exit code 139.

I'm building from the latest commit: https://github.com/signalapp/libsignal/commit/916269c3e5c9cf685099b320c33996fc2f271481

Couldn't find any open or closed issues reporting the same error.

jrose-signal commented 2 years ago

Hm, that certainly looks correct to me! What OS are you on? (including version number)

Niteo commented 2 years ago

This is on Ubuntu 20.04.4 LTS (Linux version 5.4.0-107-generic).

jrose-signal commented 2 years ago

Hm. A fresh clone on my own 20.04.4 system built fine with your instructions (except I hadn't actually installed yarn on this particular Node version, I had to npm install -g yarn after nvm use). That's not completely clean because cargo has global caches, but…

Ah, a thought: is this being run from a CLI-only shell? Maybe it's electron-mocha that's the problem, since that only works inside a windowing environment. (You can see in our GitHub workflow that we run inside xvfb for a virtual display.) In theory we could use regular mocha here; there's nothing Electron-specific, but if we're going to have automated tests they might as well match our intended use case.

Niteo commented 2 years ago

Interesting. I was running it on a remote machine, so that's probably it...!

I tried building and testing it on another machine (Debian GNU/Linux 10) with X, and then everything works. Not sure how much sense it makes to switch from mocha to electron-mocha, a warning text might just be enough.

jrose-signal commented 2 years ago

I talked with some other team members and we're going to switch to plain mocha in the next release; electron-mocha isn't adding much in practice.

Niteo commented 2 years ago

This is great news! Looking forward to the next release.

jrose-signal commented 2 years ago

Included in 0.17.0.