Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(core): use user configured npm registry #4937

Merged
merged 8 commits into from
Aug 22, 2024
9 changes: 4 additions & 5 deletions packages/core/src/initializer/index.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
import { createInjector } from 'typed-inject';
import { RestClient } from 'typed-rest-client';
import { execaCommand } from 'execa';
import { resolveFromCwd } from '@stryker-mutator/util';
import { LogLevel } from '@stryker-mutator/api/core';

import { coreTokens, provideLogger } from '../di/index.js';

import { LogConfigurator } from '../logging/log-configurator.js';
import { LogConfigurator } from '../logging/index.js';

import * as initializerTokens from './initializer-tokens.js';
import { NpmClient } from './npm-client.js';
Expand All @@ -15,14 +14,14 @@ import { StrykerInitializer } from './stryker-initializer.js';
import { StrykerInquirer } from './stryker-inquirer.js';
import { createInitializers } from './custom-initializers/index.js';
import { GitignoreWriter } from './gitignore-writer.js';

const NPM_REGISTRY = 'https://registry.npmjs.com';
import { createNpmRegistryClient, getRegistry } from './npm-registry.js';

export function initializerFactory(): StrykerInitializer {
LogConfigurator.configureMainProcess(LogLevel.Information);
return provideLogger(createInjector())
.provideValue(initializerTokens.out, console.log)
.provideValue(initializerTokens.restClientNpm, new RestClient('npm', NPM_REGISTRY))
nicojs marked this conversation as resolved.
Show resolved Hide resolved
.provideFactory(initializerTokens.npmRegistry, getRegistry)
.provideFactory(initializerTokens.restClientNpm, createNpmRegistryClient)
.provideClass(initializerTokens.npmClient, NpmClient)
.provideClass(initializerTokens.configWriter, StrykerConfigWriter)
.provideClass(initializerTokens.gitignoreWriter, GitignoreWriter)
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/initializer/initializer-tokens.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
export const restClientNpm = 'restClientNpm';
export const npmClient = 'npmClient';
export const npmRegistry = 'npmRegistry';
export const customInitializers = 'strykerPresets';
export const configWriter = 'configWriter';
export const gitignoreWriter = 'gitignoreWriter';
Expand Down
5 changes: 3 additions & 2 deletions packages/core/src/initializer/npm-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,11 @@ const handleResult =
};

export class NpmClient {
public static inject = tokens(commonTokens.logger, initializerTokens.restClientNpm);
public static inject = tokens(commonTokens.logger, initializerTokens.restClientNpm, initializerTokens.npmRegistry);
constructor(
private readonly log: Logger,
private readonly innerNpmClient: RestClient,
private readonly npmRegistry: string,
) {}

public getTestRunnerOptions(): Promise<PromptOption[]> {
Expand Down Expand Up @@ -65,7 +66,7 @@ export class NpmClient {
const response = await this.innerNpmClient.get<NpmSearchResult>(path);
return handleResult(path)(response);
} catch (err) {
this.log.error(`Unable to reach 'https://registry.npmjs.com' (for query ${path}). Please check your internet connection.`, errorToString(err));
this.log.error(`Unable to reach '${this.npmRegistry}' (for query ${path}). Please check your internet connection.`, errorToString(err));
const result: NpmSearchResult = {
objects: [],
total: 0,
Expand Down
41 changes: 41 additions & 0 deletions packages/core/src/initializer/npm-registry.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { execaSync } from 'execa';
import { RestClient } from 'typed-rest-client';
import * as initializerTokens from './initializer-tokens.js';
import { errorToString } from '@stryker-mutator/util';
import { Logger } from '@stryker-mutator/api/logging';
import { commonTokens } from '@stryker-mutator/api/plugin';

const DEFAULT_NPM_REGISTRY = 'https://registry.npmjs.com';

function getRegistry(logger: Logger): string {
if (process.env.npm_config_registry) {
return process.env.npm_config_registry;
} else if (process.env.npm_command) {
// if running inside npm and not having the registry than it's the default one
return DEFAULT_NPM_REGISTRY;
} else {
// Using global as when trying to get the registry inside npm workspace it would fail
try {
const registry = execaSync('npm', ['config', 'get', '--global', 'registry'], {
stdout: 'pipe',
timeout: 20000,
});

return registry.stdout.trim();
} catch (e) {
logger.warn(`Could not run \`npm config get --global registry\` falling back to default npm registry.`, errorToString(e));

return DEFAULT_NPM_REGISTRY;
}
}
}

getRegistry.inject = [commonTokens.logger] as const;

function createNpmRegistryClient(npmRegistry: string): RestClient {
return new RestClient('npm', npmRegistry);
}

createNpmRegistryClient.inject = [initializerTokens.npmRegistry] as const;

export { createNpmRegistryClient, getRegistry };
44 changes: 44 additions & 0 deletions packages/core/test/unit/initializer/npm-registry.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import { getRegistry } from '../../../src/initializer/npm-registry.js';
import { expect } from 'chai';
import log4js from 'log4js';

const DEFAULT_REGISTRY = 'https://registry.npmjs.com';

describe('npm registry', () => {
describe('get registry', () => {
let oldNpmConfigRegistry: string | undefined;
let oldNpmCommand: string | undefined;

before(() => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be beforeEach instead of before (or change afterEach to be after)

oldNpmConfigRegistry = process.env.npm_config_registry;

oldNpmCommand = process.env.npm_command;
});

after(() => {
process.env.npm_config_registry = oldNpmConfigRegistry;

process.env.npm_command = oldNpmCommand;
});

it('should return default repository when run with npm command with no custom repository', () => {
process.env.npm_config_registry = '';

process.env.npm_command = 'value';

const registry = getRegistry(log4js.getLogger());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use testInjector.logger. It contains a logger mock that is ready to be used. You can verify log messages on it using sinon.asserts.calledOnceWithExactly(testInjector.logger.warn, '...');


expect(registry).to.equal(DEFAULT_REGISTRY);
});

it('should return default repository when run with npm command with custom repository', () => {
process.env.npm_config_registry = 'foo';

process.env.npm_command = 'value';

const registry = getRegistry(log4js.getLogger());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here


expect(registry).to.equal('foo');
});
});
nicojs marked this conversation as resolved.
Show resolved Hide resolved
});
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,11 @@ import { StrykerInitializer } from '../../../src/initializer/stryker-initializer
import { StrykerInquirer } from '../../../src/initializer/stryker-inquirer.js';
import { Mock } from '../../helpers/producers.js';
import { GitignoreWriter } from '../../../src/initializer/gitignore-writer.js';
import { SUPPORTED_CONFIG_FILE_NAMES } from '../../../src/config/config-file-formats.js';
import { SUPPORTED_CONFIG_FILE_NAMES } from '../../../src/config/index.js';
import { CustomInitializer, CustomInitializerConfiguration } from '../../../src/initializer/custom-initializers/custom-initializer.js';
import { PackageInfo } from '../../../src/initializer/package-info.js';
import { inquire } from '../../../src/initializer/inquire.js';
import { createNpmRegistryClient, getRegistry } from '../../../src/initializer/npm-registry.js';

describe(StrykerInitializer.name, () => {
let sut: StrykerInitializer;
Expand Down Expand Up @@ -55,7 +56,8 @@ describe(StrykerInitializer.name, () => {
syncBuiltinESMExports();
sut = testInjector.injector
.provideValue(initializerTokens.out, out as unknown as typeof console.log)
.provideValue(initializerTokens.restClientNpm, npmRestClient)
.provideFactory(initializerTokens.npmRegistry, getRegistry)
.provideFactory(initializerTokens.restClientNpm, createNpmRegistryClient)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is a unit test for the StrykerInitializer, I wouldn't expect the "real" getRegistry to be used here. Instead, you can keep using provideValue here for the npmRestClient sinon mock here.

Indeed, the tests are failing, see https://github.com/stryker-mutator/stryker-js/actions/runs/10341544430/job/28626286827?pr=4937

.provideClass(initializerTokens.inquirer, StrykerInquirer)
.provideClass(initializerTokens.npmClient, NpmClient)
.provideValue(initializerTokens.customInitializers, customInitializers)
Expand Down
Loading