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

fix: no json connectedStatus on skipconnection #27

Merged
merged 10 commits into from
Feb 16, 2021
26 changes: 23 additions & 3 deletions src/shared/orgListUtil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,13 @@ export class OrgListUtil {
const [nonScratchOrgs, scratchOrgs] = await Promise.all([
Promise.all(
orgs.nonScratchOrgs.map(async (fields) => {
if (flags.skipconnectionstatus) {
fields.connectedStatus = fields.connectedStatus ?? 'Unknown';
} else {
if (!flags.skipconnectionstatus) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@mshanemc is there a test to cover this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it('skipconnectionstatus', async () => {

the command is just passing that flag to orgListUtil so the test is at that level instead of on the command.

Copy link
Contributor

Choose a reason for hiding this comment

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

@mshanemc that test verifies what happens when skipconnectionstatus is true. Is there an existing test that verifies results when it is false?

Copy link
Contributor

Choose a reason for hiding this comment

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

Another way to put it, is there a way this bug could have been caught with a test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that test now asserts that --skipconnectionstatus produces nonScratchOrgs with no connectedStatus in the json

Copy link
Contributor

Choose a reason for hiding this comment

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

@mshanemc Does the test below represent verification of the correct behavior when flags.skipconnectionstatus is false?

it('readLocallyValidatedMetaConfigsGroupedByOrgType', async () => {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

when migrating this command, I tried to keep the tests on orgListUtil as close to the original as possible.

I don't like the original either, so I'm gonna fix them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

better?

// skip completely if we're skipping the connection
fields.connectedStatus = await OrgListUtil.determineConnectedStatusForNonScratchOrg(fields.username);
if (!fields.isDevHub && fields.connectedStatus === 'Connected') {
// activating DevHub setting is irreversible so don't waste time checking any org we already know is a hub
fields.isDevHub = await OrgListUtil.checkNonScratchOrgIsDevHub(fields.username);
}
}
return fields;
})
Expand Down Expand Up @@ -257,6 +260,23 @@ export class OrgListUtil {
return orgs;
}

/**
* Asks the org if it's a devHub. Because the dev hub setting can't be deactivated, only ask orgs that aren't already stored as hubs.
* This has a number of side effects, including updating the AuthInfo files and
*
* @param username org to check for devHub status
* @returns {Promise.<boolean>}
*/
public static async checkNonScratchOrgIsDevHub(username: string): Promise<boolean> {
try {
const org = await Org.create({ aliasOrUsername: username });
// true forces a server check instead of relying on AuthInfo file cache
return org.determineIfDevHubOrg(true);
} catch {
return false;
}
}

/**
* retrieves the connection info of an nonscratch org
*
Expand Down
139 changes: 80 additions & 59 deletions test/shared/orgListUtil.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/
import { $$, expect } from '@salesforce/command/lib/test';

import { AuthInfo, ConfigAggregator, fs, Aliases } from '@salesforce/core';
import { AuthInfo, ConfigAggregator, fs, Aliases, Org } from '@salesforce/core';
import { stubMethod } from '@salesforce/ts-sinon';
import { OrgListUtil } from '../../src/shared/orgListUtil';
import * as utils from '../../src/shared/utils';
Expand Down Expand Up @@ -46,8 +46,9 @@ const orgAuthConfig = {

const devHubConfigFields = {
username: '[email protected]',
isDevHub: true,
isDevHub: false, // we want to simulate updating this as part of the flow
};

const devHubConfig = {
getFields: () => devHubConfigFields,
getConnectionOptions: () => ({ accessToken: '00D!XX' }),
Expand All @@ -59,40 +60,45 @@ const fileNames = ['[email protected]', '[email protected]'];

describe('orgListUtil tests', () => {
const spies = new Map();
let readAuthFilesStub: sinon.SinonStub;
let groupOrgsStub: sinon.SinonStub;
let aliasListStub: sinon.SinonStub;
let determineConnectedStatusForNonScratchOrg: sinon.SinonStub;
let retrieveScratchOrgInfoFromDevHubStub: sinon.SinonStub;
let checkNonScratchOrgIsDevHub: sinon.SinonStub;

describe('readLocallyValidatedMetaConfigsGroupedByOrgType', () => {
afterEach(() => spies.clear());

beforeEach(() => {
readAuthFilesStub = stubMethod($$.SANDBOX, OrgListUtil, 'readAuthFiles').resolves([
orgAuthConfig,
expiredAuthConfig,
devHubConfig,
]);
$$.SANDBOX.stub(AuthInfo, 'create');

groupOrgsStub = stubMethod($$.SANDBOX, OrgListUtil, 'groupOrgs').resolves({
nonScratchOrgs: [devHubConfigFields],
scratchOrgs: [orgAuthConfigFields, expiredAuthConfigFields],
});
stubMethod($$.SANDBOX, OrgListUtil, 'readAuthFiles').resolves([orgAuthConfig, expiredAuthConfig, devHubConfig]);
aliasListStub = stubMethod($$.SANDBOX, Aliases, 'fetch').resolves();
determineConnectedStatusForNonScratchOrg = stubMethod(
$$.SANDBOX,
OrgListUtil,
'determineConnectedStatusForNonScratchOrg'
).resolves({});
).resolves('Connected');
retrieveScratchOrgInfoFromDevHubStub = stubMethod(
$$.SANDBOX,
OrgListUtil,
'retrieveScratchOrgInfoFromDevHub'
).resolves([]);
checkNonScratchOrgIsDevHub = stubMethod($$.SANDBOX, OrgListUtil, 'checkNonScratchOrgIsDevHub').resolves(true);

spies.set('reduceScratchOrgInfo', $$.SANDBOX.spy(OrgListUtil, 'reduceScratchOrgInfo'));
stubMethod($$.SANDBOX, ConfigAggregator, 'create').resolves({
getConfig: () => {
return {
defaultusername: orgAuthConfigFields.username,
defaultdevhubusername: devHubConfigFields.username,
};
},
});

$$.SANDBOX.stub(fs, 'readFileSync');
stubMethod($$.SANDBOX, fs, 'stat').resolves({ atime: 'test' });

$$.SANDBOX.stub(utils, 'getAliasByUsername').withArgs('[email protected]').resolves('gaz');
});

afterEach(async () => {
Expand All @@ -101,21 +107,44 @@ describe('orgListUtil tests', () => {

it('readLocallyValidatedMetaConfigsGroupedByOrgType', async () => {
const flags = {};
await OrgListUtil.readLocallyValidatedMetaConfigsGroupedByOrgType(fileNames, flags);
expect(readAuthFilesStub.calledOnce).to.be.true;
expect(groupOrgsStub.calledOnce).to.be.true;
const orgs = await OrgListUtil.readLocallyValidatedMetaConfigsGroupedByOrgType(fileNames, flags);
expect(orgs.nonScratchOrgs.every((nonScratchOrg) => nonScratchOrg.connectedStatus !== undefined)).to.be.true;
expect(orgs.scratchOrgs.length).to.equal(2);
expect(orgs.scratchOrgs[0]).to.haveOwnProperty('username').to.equal('[email protected]');
expect(orgs.nonScratchOrgs.length).to.equal(1);

// devhub is updated to be true
expect(checkNonScratchOrgIsDevHub.called).to.be.true;
expect(orgs.nonScratchOrgs[0].isDevHub).to.be.true;

expect(aliasListStub.calledOnce).to.be.false;
expect(determineConnectedStatusForNonScratchOrg.calledOnce).to.be.true;
expect(retrieveScratchOrgInfoFromDevHubStub.calledOnce).to.be.true;
});

it('skipconnectionstatus', async () => {
const flags = { skipconnectionstatus: true };
await OrgListUtil.readLocallyValidatedMetaConfigsGroupedByOrgType(fileNames, flags);
expect(readAuthFilesStub.calledOnce).to.be.true;
expect(groupOrgsStub.calledOnce).to.be.true;
const orgs = await OrgListUtil.readLocallyValidatedMetaConfigsGroupedByOrgType(fileNames, flags);

// we didn't check the status, so the hub is still not known to be a devhub
expect(orgs.nonScratchOrgs[0].isDevHub).to.be.false;
expect(checkNonScratchOrgIsDevHub.called).to.be.false;

expect(orgs.nonScratchOrgs.every((nonScratchOrg) => nonScratchOrg.connectedStatus === undefined)).to.be.true;

expect(aliasListStub.calledOnce).to.be.false;
expect(determineConnectedStatusForNonScratchOrg.calledOnce).to.be.false;
expect(aliasListStub.calledOnce).to.be.false;
expect(determineConnectedStatusForNonScratchOrg.called).to.be.false;
});

it('should omit sensitive information and catergorise active and non-active scracth orgs', async () => {
const flags = {};
const orgs = await OrgListUtil.readLocallyValidatedMetaConfigsGroupedByOrgType(fileNames, flags);

expect(orgs.scratchOrgs[0]).to.not.haveOwnProperty('clientSecret');
expect(orgs.scratchOrgs[1]).to.not.haveOwnProperty('clientSecret');
expect(orgs.scratchOrgs[0]).to.not.haveOwnProperty('refreshToken');
expect(orgs.scratchOrgs[1]).to.not.haveOwnProperty('refreshToken');
});

it('should execute queries to check for org information if --verbose is used', async () => {
Expand Down Expand Up @@ -147,50 +176,42 @@ describe('orgListUtil tests', () => {
expect(retrieveScratchOrgInfoFromDevHubStub.calledOnce).to.be.true;
expect(spies.get('reduceScratchOrgInfo').calledOnce).to.be.true;
expect(orgGroups.scratchOrgs[0].signupUsername).to.equal(orgAuthConfigFields.username);
});
});

describe('groupOrgs', () => {
let contents;

beforeEach(() => {
$$.SANDBOX.stub(AuthInfo, 'create');
$$.SANDBOX.stub(utils, 'getAliasByUsername').withArgs('[email protected]').resolves('gaz');
readAuthFilesStub = stubMethod($$.SANDBOX, OrgListUtil, 'readAuthFiles').resolves([
orgAuthConfig,
expiredAuthConfig,
devHubConfig,
// current default on every scratch org, warning in v51, deprecation in v52
expect(orgGroups.scratchOrgs[0].connectedStatus).to.equal('Unknown');
expect(orgGroups.scratchOrgs[0]).to.include.keys([
'signupUsername',
'createdBy',
'createdDate',
'devHubOrgId',
'orgName',
'edition',
'status',
'expirationDate',
'isExpired',
]);
stubMethod($$.SANDBOX, fs, 'stat').resolves({ atime: 'test' });
stubMethod($$.SANDBOX, ConfigAggregator, 'create').resolves({
getConfig: () => {
return {
defaultusername: orgAuthConfigFields.username,
defaultdevhubusername: devHubConfigFields.username,
};
},
});
});

afterEach(async () => {
$$.SANDBOX.restore();
it('handles connection errors for non-scratch orgs', async () => {
determineConnectedStatusForNonScratchOrg.restore();
stubMethod($$.SANDBOX, Org, 'create').returns(Org.prototype);
stubMethod($$.SANDBOX, Org.prototype, 'getField').returns(undefined);
stubMethod($$.SANDBOX, Org.prototype, 'getUsername').returns(devHubConfigFields.username);
stubMethod($$.SANDBOX, Org.prototype, 'refreshAuth').rejects({ message: 'bad auth' });

const orgGroups = await OrgListUtil.readLocallyValidatedMetaConfigsGroupedByOrgType(fileNames, {});
expect(orgGroups.nonScratchOrgs).to.have.length(1);
expect(orgGroups.nonScratchOrgs[0].connectedStatus).to.equal('bad auth');
expect(checkNonScratchOrgIsDevHub.called).to.be.false;
});

it('ensure the auth infos are categorized into scratchOrgs, nonScratchOrgs', async () => {
contents = await OrgListUtil.readAuthFiles(['[email protected]']);
const orgs = await OrgListUtil.groupOrgs(contents);
expect(orgs.scratchOrgs.length).to.equal(2);
expect(orgs.scratchOrgs[0]).to.haveOwnProperty('username').to.equal('[email protected]');
expect(orgs.nonScratchOrgs.length).to.equal(1);
});
it('handles auth file problems for non-scratch orgs', async () => {
determineConnectedStatusForNonScratchOrg.restore();
stubMethod($$.SANDBOX, Org, 'create').rejects({ message: 'bad file' });

it('should omit sensitive information and catergorise active and non-active scracth orgs', async () => {
const authInfos = await OrgListUtil.readAuthFiles(['[email protected]']);
const orgs = await OrgListUtil.groupOrgs(authInfos);
expect(orgs.scratchOrgs[0]).to.not.haveOwnProperty('clientSecret');
expect(orgs.scratchOrgs[1]).to.not.haveOwnProperty('clientSecret');
expect(orgs.scratchOrgs[0]).to.not.haveOwnProperty('refreshToken');
expect(orgs.scratchOrgs[1]).to.not.haveOwnProperty('refreshToken');
const orgGroups = await OrgListUtil.readLocallyValidatedMetaConfigsGroupedByOrgType(fileNames, {});
expect(orgGroups.nonScratchOrgs).to.have.length(1);
expect(orgGroups.nonScratchOrgs[0].connectedStatus).to.equal('bad file');
expect(checkNonScratchOrgIsDevHub.called).to.be.false;
});
});
});