Skip to content

Commit

Permalink
test: Workaround for Jest dynamic import issues
Browse files Browse the repository at this point in the history
Dynamic imports cause segmentation faults with Jest:
nodejs/node#35889.
We work around this by handling imports in IdentityProviderFactory
differently when Jest is running.
For unit tests we use a different tsconfig
that transpiles dynamic imports differently,
as those are also used in AppRunner.
  • Loading branch information
joachimvh committed Oct 6, 2023
1 parent 5c6a140 commit 41a6e4b
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 4 deletions.
2 changes: 1 addition & 1 deletion jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ const esModules = [
module.exports = {
transform: {
'^.+\\.ts$': [ 'ts-jest', {
tsconfig: '<rootDir>/tsconfig.json',
tsconfig: '<rootDir>/test/tsconfig.json',
diagnostics: false,
isolatedModules: true,
}],
Expand Down
18 changes: 16 additions & 2 deletions src/identity/configuration/IdentityProviderFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,23 @@ export class IdentityProviderFactory implements ProviderFactory {
// Render errors with our own error handler
this.configureErrors(config);

// Allow provider to interpret reverse proxy headers.
// As oidc-provider is an ESM package and CSS is CJS, we have to use a dynamic import here.
const provider = new (await import('oidc-provider')).default(this.baseUrl, config);
// Unfortunately, there is a Node/Jest bug that causes segmentation faults when doing such an import in Jest:
// https://github.com/nodejs/node/issues/35889
// To work around that, we do the import differently, in case we are in a Jest test run.
// This can be detected via the env variables: https://jestjs.io/docs/environment-variables.
// There have been reports of `JEST_WORKER_ID` being undefined, so to be sure we check both.
let ctr: { default: new(issuer: string, configuration?: Configuration) => Provider };
// eslint-disable-next-line no-process-env
if (process.env.JEST_WORKER_ID ?? process.env.NODE_ENV === 'test') {
// eslint-disable-next-line no-undef
ctr = jest.requireActual('oidc-provider');
} else {
ctr = await import('oidc-provider');
}
const provider = new ctr.default(this.baseUrl, config);

// Allow provider to interpret reverse proxy headers.
provider.proxy = true;

this.captureErrorResponses(provider);
Expand Down
3 changes: 3 additions & 0 deletions test/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
{
"extends": "../tsconfig.json",
"compilerOptions": {
"moduleResolution": "node"
},
"include": [
"."
]
Expand Down
28 changes: 27 additions & 1 deletion test/unit/identity/configuration/IdentityProviderFactory.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ const routes = {
};

describe('An IdentityProviderFactory', (): void => {
let jestWorkerId: string | undefined;
let nodeEnv: string | undefined;
let baseConfig: Configuration;
const baseUrl = 'http://example.com/foo/';
const oidcPath = '/oidc';
Expand All @@ -53,8 +55,24 @@ describe('An IdentityProviderFactory', (): void => {
let responseWriter: jest.Mocked<ResponseWriter>;
let factory: IdentityProviderFactory;

beforeAll(async(): Promise<void> => {
// We need to fool the IDP factory into thinking we are not in a test run,
// otherwise we can't mock the oidc-provider library due to the workaround in the code there.
jestWorkerId = process.env.JEST_WORKER_ID;
nodeEnv = process.env.NODE_ENV;
delete process.env.JEST_WORKER_ID;
delete process.env.NODE_ENV;
});

afterAll(async(): Promise<void> => {
process.env.JEST_WORKER_ID = jestWorkerId;
process.env.NODE_ENV = nodeEnv;
});

beforeEach(async(): Promise<void> => {
baseConfig = { claims: { webid: [ 'webid', 'client_webid' ]}};
// Disabling devInteractions to prevent warnings when testing the path
// where we use the actual library instead of a mock.
baseConfig = { claims: { webid: [ 'webid', 'client_webid' ]}, features: { devInteractions: { enabled: false }}};

ctx = {
method: 'GET',
Expand Down Expand Up @@ -308,4 +326,12 @@ describe('An IdentityProviderFactory', (): void => {
expect(oldAccept).toHaveBeenCalledTimes(1);
expect(oldAccept).toHaveBeenLastCalledWith('something');
});

it('avoids dynamic imports when testing with Jest.', async(): Promise<void> => {
// Reset the env variable, so we can test the path where the dynamic import is not used
process.env.JEST_WORKER_ID = jestWorkerId;
const provider = await factory.getProvider() as any;
// We don't define this in our mock
expect(provider.app).toBeDefined();
});
});

0 comments on commit 41a6e4b

Please sign in to comment.