Repository.open doesn't resolve in some cases #1679
Comments
|
It works well with mocha |
|
I am getting this same issue. Could not consistently reproduce on my MacBook, but can consistently (100%) reproduce within a Anyone manage to resolve this? |
|
This is caused by jestjs/jest#3552. I recently experienced the same problem after adding a 9th testsuite on a quad-core hyperthreaded machine, therefore having 8 threads, and that let me down the rabbit hole. In the end we managed to find the culprit and I now fully understand why this issue occurs. First some context on how Jest works, as it is relevant in understanding the problem. ExplanationJest has its own module resolution logic to support mocking modules, and it evicts the module caches between running testsuites. By default, it will spawn N processes, with N being equal to the number of threads you have available. Therefore, as long as there's fewer testsuites than the number of threads, each process runs only a single testsuite. Once you start having more testsuites than threads, or run Jest with the Unfortunately, the initial load of the nodegit/generate/templates/templates/nodegit.js Lines 19 to 23 in 37fcf5d What I found is that this Now, when Jest loads the WorkaroundsI found essentially two workarounds for the issue, as follows:
There doesn't appear to be any configuration options to achieve this, it requires a patch inside of - const EXPERIMENTAL_MODULES = ['worker_threads'];
+ const EXPERIMENTAL_MODULES = ['worker_threads', 'nodegit'];
Then, create the // In order to patch jest's Runtime class, we need to require it using NodeJS's require function, *not* the require
// function that is provided by jest here. The reason is that using jest's require will load the Runtime code a second
// time, so that we won't get a reference to the actual instance that needs to be patched. Since jest does not expose
// any API to obtain the native NodeJS require function, we load the native "module" module instead that allows us to
// create a native NodeJS require function. Mind is blown!
const Module = require('module');
const nativeRequire = Module.createRequireFromPath(__filename);
const jestRuntime = nativeRequire('jest-runtime');
if (!jestRuntime.prototype.__nodegit_patched) {
jestRuntime.prototype.__nodegit_patched = true;
const originalRequireModule = jestRuntime.prototype.requireModule;
jestRuntime.prototype.requireModule = function(from, moduleName) {
// When requiring the "nodegit" module, sidestep jest's module system as it would cause nodegit to be loaded
// multiple times (once per test file), corrupting the bound functions to native code.
// See https://github.com/facebook/jest/issues/3552
if (moduleName === 'nodegit') {
return nativeRequire('nodegit');
}
return originalRequireModule.apply(this, arguments);
};
}Essentially what this does is patch the Jest runtime to rewire module resolution to the native NodeJS module loader, but only if the module to load is DisclaimerWith both workarounds, you'll end up in a situation where Wrapping upI suspect that from the From the Jest side of the equation, I think it could be very valuable to be able to configure the module loader in some ways. Either its module loader should be able to disable altogether, or it should be possible for specific modules to sidestep its module loader. Bottom line, this was one of the most annoying bugs I have ever had to debug. I haven't had the time to report issues, if anyone feels like doing so please feel free to link to this post. |
|
@JoostK Thank you so much for the thorough break-down of this bug. It took me forever to track down that my test suite was hanging on a repo init, and once I found the culprit, I wouldn't have guessed how to solve it, by far. |
@bhubr It's not Jest that is hanging, it's somewhere in the nodegit-promise library. What I found during debugging was that the double promisification seems to introduce some internal state that causes the promise resolution to get stuck. I don't know the exact details as I'm not really familiar with its internals.
Oh well, this took me 4 hours to figure out and fix so it was a pretty deep rabbit hole. |
Three unit tests were frequently timing out, for reasons possibly documented in nodegit/nodegit#1679. Changed these tests to integration tests to avoid CI failures. They can be run selectively instead.
Three unit tests were frequently timing out, for reasons possibly documented in nodegit/nodegit#1679. Changed these tests to integration tests to avoid CI failures. They can be run selectively instead.
Three unit tests were frequently timing out, for reasons possibly documented in nodegit/nodegit#1679. Changed these tests to integration tests to avoid CI failures. They can be run selectively instead.
Three unit tests were frequently timing out, for reasons possibly documented in nodegit/nodegit#1679. Changed these tests to integration tests to avoid CI failures. They can be run selectively instead.
|
I've been banging my head against this for a few hours now, unfortunately the workarounds suggested don't seem to work any more. In todays version of jest that monkey patch is never called. I thought I could mock my way around this by mocking out |
|
@tommoor Yeah, the |
|
@JoostK you have a private fork of Jest just for this? 😭 |
Nope just patching |
|
I see, thank you, unfortunately this workaround doesn't seem to work when setting |
System information
Here is the example: https://github.com/Means88/nodegit-test
It contains 5 same tests
And if I run jest with
yarn test --maxWorkers 4, there will be 4 test cases passed and 1 test case failed. But it will pass if I open the repository several times in one test case likeMaybe there are something like mutex lock and didn't clean up after test? I've tried
enableThreadSafetyandsetThreadSafetyStatusbut nothing changed.The text was updated successfully, but these errors were encountered: