Skip to content

Commit

Permalink
fix(core): should use nx cloud if access token specified by env (#19975)
Browse files Browse the repository at this point in the history
  • Loading branch information
AgentEnder authored Nov 8, 2023
1 parent 9330513 commit 4f23523
Show file tree
Hide file tree
Showing 10 changed files with 136 additions and 60 deletions.
10 changes: 0 additions & 10 deletions e2e/nx-misc/src/misc.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,6 @@ describe('migrate', () => {
'migrate migrate-parent-package@2.0.0 --from="migrate-parent-package@1.0.0"',
{
env: {
...process.env,
NX_MIGRATE_SKIP_INSTALL: 'true',
NX_MIGRATE_USE_LOCAL: 'true',
},
Expand Down Expand Up @@ -479,7 +478,6 @@ describe('migrate', () => {
// runs migrations
runCLI('migrate --run-migrations=migrations.json', {
env: {
...process.env,
NX_MIGRATE_SKIP_INSTALL: 'true',
NX_MIGRATE_USE_LOCAL: 'true',
},
Expand All @@ -493,7 +491,6 @@ describe('migrate', () => {
'migrate migrate-parent-package@2.0.0 --from="migrate-parent-package@1.0.0"',
{
env: {
...process.env,
NX_MIGRATE_SKIP_INSTALL: 'true',
NX_MIGRATE_USE_LOCAL: 'true',
},
Expand All @@ -503,7 +500,6 @@ describe('migrate', () => {
// runs migrations with createCommits enabled
runCLI('migrate --run-migrations=migrations.json --create-commits', {
env: {
...process.env,
NX_MIGRATE_SKIP_INSTALL: 'true',
NX_MIGRATE_USE_LOCAL: 'true',
},
Expand All @@ -522,7 +518,6 @@ describe('migrate', () => {
'migrate migrate-parent-package@2.0.0 --from="migrate-parent-package@1.0.0"',
{
env: {
...process.env,
NX_MIGRATE_SKIP_INSTALL: 'true',
NX_MIGRATE_USE_LOCAL: 'true',
},
Expand All @@ -534,7 +529,6 @@ describe('migrate', () => {
`migrate --run-migrations=migrations.json --create-commits --commit-prefix="'chore(core): AUTOMATED - '"`,
{
env: {
...process.env,
NX_MIGRATE_SKIP_INSTALL: 'true',
NX_MIGRATE_USE_LOCAL: 'true',
},
Expand All @@ -553,7 +547,6 @@ describe('migrate', () => {
'migrate migrate-parent-package@2.0.0 --from="migrate-parent-package@1.0.0"',
{
env: {
...process.env,
NX_MIGRATE_SKIP_INSTALL: 'true',
NX_MIGRATE_USE_LOCAL: 'true',
},
Expand All @@ -565,7 +558,6 @@ describe('migrate', () => {
`migrate --run-migrations=migrations.json --commit-prefix CUSTOM_PREFIX`,
{
env: {
...process.env,
NX_MIGRATE_SKIP_INSTALL: 'true',
NX_MIGRATE_USE_LOCAL: 'true',
},
Expand All @@ -584,7 +576,6 @@ describe('migrate', () => {
// Invalid: runs migrations with a custom commit-prefix but without enabling --create-commits
const output = runCLI(`migrate --run-migrations`, {
env: {
...process.env,
NX_MIGRATE_SKIP_INSTALL: 'true',
NX_MIGRATE_USE_LOCAL: 'true',
},
Expand All @@ -602,7 +593,6 @@ describe('migrate', () => {
// Invalid: runs migrations with a custom commit-prefix but without enabling --create-commits
const output = runCLI(`migrate --run-migrations --if-exists`, {
env: {
...process.env,
NX_MIGRATE_SKIP_INSTALL: 'true',
NX_MIGRATE_USE_LOCAL: 'true',
},
Expand Down
1 change: 0 additions & 1 deletion e2e/nx-run/src/nx-cloud.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ describe('Nx Cloud', () => {
beforeAll(() => {
runCLI('connect --no-interactive', {
env: {
...process.env,
NX_CLOUD_API: 'https://staging.nx.app',
},
});
Expand Down
6 changes: 3 additions & 3 deletions e2e/nx-run/src/run.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -331,13 +331,13 @@ describe('Nx Running Tests', () => {
runCLI(`generate @nx/web:app ${myapp}`);

const buildWithDaemon = runCLI(`build ${myapp}`, {
env: { ...process.env, NX_DAEMON: 'false' },
env: { NX_DAEMON: 'false' },
});

expect(buildWithDaemon).toContain('Successfully ran target build');

const buildAgain = runCLI(`build ${myapp}`, {
env: { ...process.env, NX_DAEMON: 'false' },
env: { NX_DAEMON: 'false' },
});

expect(buildAgain).toContain('[local cache]');
Expand Down Expand Up @@ -614,7 +614,7 @@ describe('Nx Running Tests', () => {

// testing run many with daemon disabled
const buildWithDaemon = runCLI(`run-many --target=build`, {
env: { ...process.env, NX_DAEMON: 'false' },
env: { NX_DAEMON: 'false' },
});
expect(buildWithDaemon).toContain(`Successfully ran target build`);
}, 1000000);
Expand Down
1 change: 0 additions & 1 deletion e2e/web/src/web.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,6 @@ describe('CLI - Environment Variables', () => {
`run-many --target build --outputHashing=none --optimization=false`,
{
env: {
...process.env,
NODE_ENV: 'test',
NX_BUILD: '52',
NX_API: 'QA',
Expand Down
115 changes: 82 additions & 33 deletions packages/nx/src/command-line/connect/connect-to-nx-cloud.spec.ts
Original file line number Diff line number Diff line change
@@ -1,69 +1,118 @@
import { withEnvironmentVariables } from '../../internal-testing-utils/with-environment';
import { onlyDefaultRunnerIsUsed } from './connect-to-nx-cloud';

describe('connect-to-nx-cloud', () => {
describe('onlyDefaultRunnerIsUsed', () => {
it('should say no if tasks runner options is undefined and nxCloudAccessToken is set', () => {
expect(
onlyDefaultRunnerIsUsed({
nxCloudAccessToken: 'xxx-xx-xxx',
})
withEnvironmentVariables(
{
NX_CLOUD_ACCESS_TOKEN: null,
},
() =>
onlyDefaultRunnerIsUsed({
nxCloudAccessToken: 'xxx-xx-xxx',
})
)
).toBe(false);
});

it('should say no if cloud access token is in env', () => {
const defaultRunnerUsed = withEnvironmentVariables(
{
NX_CLOUD_ACCESS_TOKEN: 'xxx-xx-xxx',
},
() => onlyDefaultRunnerIsUsed({})
);

expect(defaultRunnerUsed).toBe(false);
});

it('should say yes if tasks runner options is undefined and nxCloudAccessToken is not set', () => {
expect(onlyDefaultRunnerIsUsed({})).toBe(true);
expect(
withEnvironmentVariables(
{
NX_CLOUD_ACCESS_TOKEN: null,
},
() => onlyDefaultRunnerIsUsed({})
)
).toBe(true);
});

it('should say yes if tasks runner options is set to default runner', () => {
expect(
onlyDefaultRunnerIsUsed({
tasksRunnerOptions: {
default: {
runner: 'nx/tasks-runners/default',
},
withEnvironmentVariables(
{
NX_CLOUD_ACCESS_TOKEN: null,
},
})
() =>
onlyDefaultRunnerIsUsed({
tasksRunnerOptions: {
default: {
runner: 'nx/tasks-runners/default',
},
},
})
)
).toBeTruthy();
});

it('should say no if tasks runner is set to a custom runner', () => {
expect(
onlyDefaultRunnerIsUsed({
tasksRunnerOptions: {
default: {
runner: 'custom-runner',
},
withEnvironmentVariables(
{
NX_CLOUD_ACCESS_TOKEN: null,
},
})
() =>
onlyDefaultRunnerIsUsed({
tasksRunnerOptions: {
default: {
runner: 'custom-runner',
},
},
})
)
).toBeFalsy();
});

it('should say yes if tasks runner has options, but no runner and not using cloud', () => {
expect(
onlyDefaultRunnerIsUsed({
tasksRunnerOptions: {
default: {
options: {
foo: 'bar',
},
},
withEnvironmentVariables(
{
NX_CLOUD_ACCESS_TOKEN: null,
},
})
() =>
onlyDefaultRunnerIsUsed({
tasksRunnerOptions: {
default: {
options: {
foo: 'bar',
},
},
},
})
)
).toBeTruthy();
});

it('should say no if tasks runner has options, but no runner and using cloud', () => {
expect(
onlyDefaultRunnerIsUsed({
tasksRunnerOptions: {
default: {
options: {
foo: 'bar',
},
},
withEnvironmentVariables(
{
NX_CLOUD_ACCESS_TOKEN: null,
},
nxCloudAccessToken: 'xxx-xx-xxx',
})
() =>
onlyDefaultRunnerIsUsed({
tasksRunnerOptions: {
default: {
options: {
foo: 'bar',
},
},
},
nxCloudAccessToken: 'xxx-xx-xxx',
})
)
).toBeFalsy();
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export function onlyDefaultRunnerIsUsed(nxJson: NxJsonConfiguration) {
// No tasks runner options OR no default runner defined:
// - If access token defined, uses cloud runner
// - If no access token defined, uses default
return !nxJson.nxCloudAccessToken;
return !(nxJson.nxCloudAccessToken ?? process.env.NX_CLOUD_ACCESS_TOKEN);
}

return defaultRunner === 'nx/tasks-runners/default';
Expand Down
18 changes: 12 additions & 6 deletions packages/nx/src/internal-testing-utils/with-environment.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
export function withEnvironmentVariables(
env: Record<string, string>,
callback: () => void | Promise<void>
): void | Promise<void> {
export function withEnvironmentVariables<T>(
env: Record<string, string | false | null | undefined>,
callback: () => T
): T {
const originalValues: Record<string, string> = {};
for (const key in env) {
originalValues[key] = process.env[key];
process.env[key] = env[key];
const value = env[key];
if (value) {
process.env[key] = value;
} else {
delete process.env[key];
}
}
const cleanup = () => {
for (const key in env) {
Expand All @@ -14,8 +19,9 @@ export function withEnvironmentVariables(
};
const p = callback();
if (p instanceof Promise) {
return p.finally(cleanup);
return p.finally(cleanup) as T;
} else {
cleanup();
return p;
}
}
18 changes: 17 additions & 1 deletion packages/nx/src/tasks-runner/run-command.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { getRunner } from './run-command';
import { NxJsonConfiguration } from '../config/nx-json';
import { join } from 'path';
import { nxCloudTasksRunnerShell } from '../nx-cloud/nx-cloud-tasks-runner-shell';
import { withEnvironmentVariables } from '../internal-testing-utils/with-environment';

describe('getRunner', () => {
let nxJson: NxJsonConfiguration;
Expand Down Expand Up @@ -77,7 +78,12 @@ describe('getRunner', () => {
it('uses default runner when no tasksRunnerOptions are present', () => {
jest.mock(join(__dirname, './default-tasks-runner.ts'), () => mockRunner);

const { tasksRunner } = getRunner({}, {});
const { tasksRunner } = withEnvironmentVariables(
{
NX_CLOUD_ACCESS_TOKEN: undefined,
},
() => getRunner({}, {})
);

expect(tasksRunner).toEqual(mockRunner);
});
Expand All @@ -100,6 +106,16 @@ describe('getRunner', () => {
`);
});

it('uses cloud runner when tasksRunnerOptions are not present and accessToken is set in env', () => {
const { tasksRunner } = withEnvironmentVariables(
{
NX_CLOUD_ACCESS_TOKEN: 'xxx-xx-xxx',
},
() => getRunner({}, {})
);
expect(tasksRunner).toEqual(nxCloudTasksRunnerShell);
});

it('reads options from base properties if no runner options provided', () => {
jest.mock(join(__dirname, './default-tasks-runner.ts'), () => mockRunner);

Expand Down
8 changes: 7 additions & 1 deletion packages/nx/src/tasks-runner/run-command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,9 @@ function getTasksRunnerPath(
// No tasksRunnerOptions for given --runner
nxJson.nxCloudAccessToken ||
// No runner prop in tasks runner options, check if access token is set.
nxJson.tasksRunnerOptions?.[runner]?.options?.accessToken;
nxJson.tasksRunnerOptions?.[runner]?.options?.accessToken ||
// Cloud access token specified in env var.
process.env.NX_CLOUD_ACCESS_TOKEN;

return isCloudRunner ? 'nx-cloud' : require.resolve('./default-tasks-runner');
}
Expand All @@ -517,6 +519,10 @@ export function getRunnerOptions(
...nxArgs,
};

// NOTE: we don't pull from env here because the cloud package
// supports it within nx-cloud's implementation. We could
// normalize it here, and that may make more sense, but
// leaving it as is for now.
if (nxJson.nxCloudAccessToken && isCloudDefault) {
result.accessToken ??= nxJson.nxCloudAccessToken;
}
Expand Down
Loading

0 comments on commit 4f23523

Please sign in to comment.