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

Wr/unset alias config #874

Merged
merged 29 commits into from
Jun 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
a06ce1f
fix: unset aliases and configs when org is deleted
WillieRuemmele Jun 26, 2023
b28ef5b
chore: flatMap change
WillieRuemmele Jun 26, 2023
992e8e5
Merge branch 'main' into wr/unsetAliasConfig
WillieRuemmele Jun 26, 2023
0436795
chore: bump jsforce
WillieRuemmele Jun 26, 2023
56b3ea1
chore: move it so that it works when deleting sandboxes as well
WillieRuemmele Jun 26, 2023
d907350
test: perf attempt
mshanemc Jun 27, 2023
e91f910
test: only perf tests for now
mshanemc Jun 27, 2023
a45fdd8
test: filename
mshanemc Jun 27, 2023
428d3b0
test: npm not yarn
mshanemc Jun 27, 2023
b04ec41
test: install deps
mshanemc Jun 27, 2023
d3a051a
test: logger specific tests
mshanemc Jun 27, 2023
ccb69d0
test: with cache
mshanemc Jun 28, 2023
8708626
test: no autopush
mshanemc Jun 28, 2023
5f9a3c2
test: new output file path
mshanemc Jun 28, 2023
edf6ad5
test: perf on windows
mshanemc Jun 28, 2023
e7258d6
style: remove example comments
mshanemc Jun 28, 2023
d706cee
test: os and name
mshanemc Jun 28, 2023
59d2994
test: revert os and name
mshanemc Jun 28, 2023
2587fdd
docs: readme and Benchmark names
mshanemc Jun 28, 2023
d62da01
test: restore all tests
mshanemc Jun 28, 2023
af4e071
test: use warn level to capture real (non-noop) traffic
mshanemc Jun 28, 2023
af8eb5d
chore: qa changes
WillieRuemmele Jun 28, 2023
b3960eb
chore: write only when needed
WillieRuemmele Jun 28, 2023
5c3abfb
test: fix UT
WillieRuemmele Jun 28, 2023
b3a3d11
test: fix UT
WillieRuemmele Jun 28, 2023
985bd0b
Merge branch 'sm/perf-metrics' into wr/unsetAliasConfig
mshanemc Jun 29, 2023
a548663
test: build before perf
mshanemc Jun 29, 2023
be3aee2
test: longer timeout for window lts-1
mshanemc Jun 29, 2023
8ebb20a
test: don't need cache
mshanemc Jun 29, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions .github/workflows/perf.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# runs some very-large-repo tests (for windows filesystem limits) and commits perf results for comparison
name: perf-tests
on:
push:
branches-ignore: [main]
workflow_dispatch:

# linux will finish ahead of windows, but prevent other branches/commits from hitting simultaneously
# since we're pushing git commits and there would be conflicts
concurrency: perf-test

jobs:
perf-tests:
strategy:
matrix:
os: [ubuntu-latest, windows-latest]
fail-fast: false
runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@v3
- uses: actions/setup-node@v3
with:
cache: yarn
- run: yarn install
- run: yarn build
- run: npm run test:perf | tee test/perf/output.txt

# Run `github-action-benchmark` action
- name: Store benchmark result
uses: benchmark-action/github-action-benchmark@5bbce78ef18edf5b96cb2d23e8d240b485f9dc4a
with:
name: Logger Benchmarks - ${{ matrix.os }}
tool: 'benchmarkjs'
output-file-path: test/perf/output.txt
comment-on-alert: true
fail-on-alert: true
# Push and deploy GitHub pages branch automatically
# this has a bug where it creates duplicate commits when summary-always and aut-push are both true
# summary-always: true
comment-always: true
benchmark-data-dir-path: perf-${{ runner.os}}
auto-push: true
github-token: ${{ secrets.SVC_CLI_BOT_GITHUB_TOKEN }}
2 changes: 1 addition & 1 deletion .mocharc.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@
"watch-files": ["src", "test"],
"recursive": true,
"reporter": "spec",
"timeout": 10000
"timeout": 20000
}
10 changes: 10 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,13 @@ In your plugin or library,
}

```

## Performance Testing

There are some benchmark.js checks to get a baseline for Logger performance.
https://forcedotcom.github.io/sfdx-core/perf-Linux
https://forcedotcom.github.io/sfdx-core/perf-Windows

You can add more test cases in test/perf/logger/main.js

If you add tests for new parts of sfdx-core outside of Logger, add new test Suites and create new jobs in the GHA `perf.yml`
9 changes: 6 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@
"prepack": "sf-prepack",
"prepare": "sf-install",
"test": "wireit",
"test:only": "wireit"
"test:only": "wireit",
"test:perf": "ts-node test/perf/logger/main.test.ts"
},
"keywords": [
"force",
Expand All @@ -49,7 +50,7 @@
"faye": "^1.4.0",
"form-data": "^4.0.0",
"js2xmlparser": "^4.0.1",
"jsforce": "^2.0.0-beta.25",
"jsforce": "^2.0.0-beta.27",
"jsonwebtoken": "9.0.0",
"jszip": "3.10.1",
"proper-lockfile": "^4.1.2",
Expand All @@ -60,6 +61,7 @@
"@salesforce/dev-scripts": "^5.4.2",
"@salesforce/prettier-config": "^0.0.3",
"@salesforce/ts-sinon": "^1.4.8",
"@types/benchmark": "^2.1.2",
"@types/chai-string": "^1.4.2",
"@types/debug": "0.0.31",
"@types/jsonwebtoken": "9.0.2",
Expand All @@ -68,6 +70,7 @@
"@types/shelljs": "0.8.12",
"@typescript-eslint/eslint-plugin": "^5.59.9",
"@typescript-eslint/parser": "^5.59.11",
"benchmark": "^2.1.4",
"chai": "^4.3.7",
"chai-string": "^1.5.0",
"eslint": "^8.43.0",
Expand Down Expand Up @@ -171,4 +174,4 @@
]
}
}
}
}
20 changes: 16 additions & 4 deletions src/config/configAggregator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ export class ConfigAggregator extends AsyncOptionalCreatable<ConfigAggregator.Op

// Initialized in loadProperties
private allowedProperties!: ConfigPropertyMeta[];
private localConfig?: Config;
private globalConfig: Config;
private readonly localConfig?: Config;
private readonly globalConfig: Config;
private envVars: Dictionary<string> = {};

/**
Expand Down Expand Up @@ -334,6 +334,19 @@ export class ConfigAggregator extends AsyncOptionalCreatable<ConfigAggregator.Op
return this.config;
}

public async unsetByValue(key: string): Promise<void> {
const hasLocalWrites = this.localConfig
?.getKeysByValue(key)
.map((k) => this.localConfig?.unset(k))
.some(Boolean);
if (hasLocalWrites) await this.localConfig?.write();
const hasGlobalWrites = this.globalConfig
?.getKeysByValue(key)
.map((k) => this.globalConfig?.unset(k))
.some(Boolean);
if (hasGlobalWrites) await this.globalConfig?.write();
}

/**
* Get the config properties that are environment variables.
*/
Expand Down Expand Up @@ -412,8 +425,7 @@ export class ConfigAggregator extends AsyncOptionalCreatable<ConfigAggregator.Op
configs.push(this.envVars);

const json: JsonMap = {};
const reduced = configs.filter(isJsonMap).reduce((acc: JsonMap, el: AnyJson) => merge(acc, el), json);
return reduced;
return configs.filter(isJsonMap).reduce((acc: JsonMap, el: AnyJson) => merge(acc, el), json);
}
}

Expand Down
11 changes: 11 additions & 0 deletions src/org/org.ts
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,16 @@ export class Org extends AsyncOptionalCreatable<Org.Options> {
* Will delete 'this' instance remotely and any files locally
*/
public async delete(): Promise<void> {
const username = ensureString(this.getUsername());

// unset any aliases referencing this org
const stateAgg = await StateAggregator.getInstance();
const existingAliases = stateAgg.aliases.getAll(username);
await stateAgg.aliases.unsetValuesAndSave(existingAliases);

// unset any configs referencing this org
[...existingAliases, username].flatMap((name) => this.configAggregator.unsetByValue(name));

if (await this.isSandbox()) {
await this.deleteSandbox();
} else {
Expand Down Expand Up @@ -1132,6 +1142,7 @@ export class Org extends AsyncOptionalCreatable<Org.Options> {
try {
const devHubConn = devHub.getConnection();
const username = this.getUsername();

const activeScratchOrgRecordId = (
await devHubConn.singleRecordQuery<{ Id: string }>(
`SELECT Id FROM ActiveScratchOrg WHERE SignupUsername='${username}'`
Expand Down
40 changes: 40 additions & 0 deletions test/perf/logger/main.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* Copyright (c) 2023, salesforce.com, inc.
* All rights reserved.
* Licensed under the BSD 3-Clause license.
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/
import { Suite } from 'benchmark';
import { Logger } from '../../../src/exported';

const suite = new Suite();

const logger = new Logger('Benchmarks');

// add tests
suite
.add('Child logger creation', () => {
// eslint-disable-next-line @typescript-eslint/prefer-includes
Logger.childFromRoot('benchmarkChild');
})
.add('Logging a string on root logger', () => {
logger.warn('this is a string');
})
.add('Logging an object on root logger', () => {
logger.warn({ foo: 1, bar: 2, baz: 3 });
})
.add('Logging an object with a message on root logger', () => {
logger.warn({ foo: 1, bar: 2, baz: 3 }, 'this is a message');
})
.add('Logging an object with a redacted prop on root logger', () => {
logger.warn({ foo: 1, bar: 2, accessToken: '00D' });
})
.add('Logging a nested 3-level object on root logger', () => {
logger.warn({ foo: 1, bar: 2, baz: { foo: 1, bar: 2, baz: { foo: 1, bar: 2, baz: 3 } } });
})
// add listeners
.on('cycle', (event: any) => {
// eslint-disable-next-line no-console, @typescript-eslint/no-unsafe-member-access
console.log(String(event.target));
})
.run({ async: true });
2 changes: 1 addition & 1 deletion test/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"extends": "@salesforce/dev-config/tsconfig-test-strict",
"include": ["unit/**/*.ts"],
"include": ["unit/**/*.ts", "perf/**/*.ts"],
"compilerOptions": {
"noEmit": true,
"skipLibCheck": true,
Expand Down
36 changes: 36 additions & 0 deletions test/unit/org/orgTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,42 @@ describe('Org Tests', () => {
}
});

it('should unset the alias and any configs', async () => {
const dev = await createOrgViaAuthInfo(testData.username);
const configAgg = await ConfigAggregator.create();

const orgTestData = new MockTestOrgData();
const org = await createOrgViaAuthInfo(orgTestData.username, orgTestData);
const username = ensureString(org.getUsername());
$$.setConfigStubContents('Config', { contents: { [OrgConfigProperties.TARGET_ORG]: username } });

await configAgg.reload();
const stateAggregator = await StateAggregator.getInstance();

expect(configAgg.getConfig()['target-org']).to.equal(username);

await stateAggregator.aliases.setAndSave('deleteThisAlias', username);
expect(stateAggregator.aliases.getUsername('deleteThisAlias')).to.equal(username);

const devHubQuery = stubMethod($$.SANDBOX, Connection.prototype, 'singleRecordQuery').resolves({
Id: orgTestData.orgId,
});
const devHubDelete = stubMethod($$.SANDBOX, Org.prototype, 'destroyScratchOrg').resolves();
$$.SANDBOX.stub(org, 'getDevHubOrg').resolves(dev);

await org.delete();
expect(configAgg.getConfig()['target-org']).to.equal(undefined);

expect(stateAggregator.aliases.get(username)).to.be.null;
expect(devHubQuery.calledOnce).to.be.true;
expect(devHubQuery.firstCall.args[0]).to.equal(
`SELECT Id FROM ActiveScratchOrg WHERE SignupUsername='${orgTestData.username}'`
);

expect(devHubDelete.calledOnce).to.be.true;
expect(devHubDelete.firstCall.args[1]).to.equal(orgTestData.orgId);
});

it('should handle SingleRecordQueryErrors.NoRecords errors', async () => {
const dev = await createOrgViaAuthInfo();

Expand Down
28 changes: 23 additions & 5 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -703,6 +703,11 @@
resolved "https://registry.yarnpkg.com/@tsconfig/node16/-/node16-1.0.3.tgz#472eaab5f15c1ffdd7f8628bd4c4f753995ec79e"
integrity sha512-yOlFc+7UtL/89t2ZhjPvvB/DeAr3r+Dq58IgzsFkOAvVC6NMJXmCGjbptdXdR9qsX7pKcTL+s87FtYREi2dEEQ==

"@types/benchmark@^2.1.2":
version "2.1.2"
resolved "https://registry.yarnpkg.com/@types/benchmark/-/benchmark-2.1.2.tgz#b7838408c93dc08ceb4e6e13147dbfbe6a151f82"
integrity sha512-EDKtLYNMKrig22jEvhXq8TBFyFgVNSPmDF2b9UzJ7+eylPqdZVo17PCUMkn1jP6/1A/0u78VqYC6VrX6b8pDWA==

"@types/chai-string@^1.4.2":
version "1.4.2"
resolved "https://registry.yarnpkg.com/@types/chai-string/-/chai-string-1.4.2.tgz#0f116504a666b6c6a3c42becf86634316c9a19ac"
Expand Down Expand Up @@ -1179,6 +1184,14 @@ base64url@^3.0.1:
resolved "https://registry.yarnpkg.com/base64url/-/base64url-3.0.1.tgz#6399d572e2bc3f90a9a8b22d5dbb0a32d33f788d"
integrity sha512-ir1UPr3dkwexU7FdV8qBBbNDRUhMmIekYMFZfi+C/sLNnRESKPl23nB9b2pltqfOQNnGzsDdId90AEtG5tCx4A==

benchmark@^2.1.4:
version "2.1.4"
resolved "https://registry.yarnpkg.com/benchmark/-/benchmark-2.1.4.tgz#09f3de31c916425d498cc2ee565a0ebf3c2a5629"
integrity sha512-l9MlfN4M1K/H2fbhfMy3B7vJd6AGKJVQn2h6Sg/Yx+KckoUA7ewS5Vv6TjSq18ooE1kS9hhAlQRH3AkXIh/aOQ==
dependencies:
lodash "^4.17.4"
platform "^1.3.3"

binary-extensions@^2.0.0:
version "2.2.0"
resolved "https://registry.yarnpkg.com/binary-extensions/-/binary-extensions-2.2.0.tgz#75f502eeaf9ffde42fc98829645be4ea76bd9e2d"
Expand Down Expand Up @@ -2976,10 +2989,10 @@ jsesc@^2.5.1:
resolved "https://registry.yarnpkg.com/jsesc/-/jsesc-2.5.2.tgz#80564d2e483dacf6e8ef209650a67df3f0c283a4"
integrity sha512-OYu7XEzjkCQ3C5Ps3QIZsQfNpqoJyZZA99wd9aWd05NCtC5pWOkShK2mkL6HXQR6/Cy2lbNdPlZBpuQHXE63gA==

jsforce@^2.0.0-beta.25:
version "2.0.0-beta.25"
resolved "https://registry.yarnpkg.com/jsforce/-/jsforce-2.0.0-beta.25.tgz#120a3999babf96ae18ab8f1232003d56588a9ca5"
integrity sha512-ZtzwJErI4SSJYWrGAw0mHEHPZRB4Idz0RiXHakCtEgEjEWt6JIDR4sNbWRHUzWHdEO4O61z2YSBvdOuag1hkWg==
jsforce@^2.0.0-beta.27:
version "2.0.0-beta.27"
resolved "https://registry.yarnpkg.com/jsforce/-/jsforce-2.0.0-beta.27.tgz#ba822b18b77bea4529491060a9b07bc1009735ac"
integrity sha512-d9dDWWeHwayRKPo8FJBAIUyk8sNXGSHwdUjR6al3yK0YKci27Jc1XfNaQTxEAuymHQJVaCb1gxTKqmA4uznFdQ==
dependencies:
"@babel/runtime" "^7.12.5"
"@babel/runtime-corejs3" "^7.12.5"
Expand Down Expand Up @@ -3201,7 +3214,7 @@ lodash.upperfirst@^4.3.1:
resolved "https://registry.yarnpkg.com/lodash.upperfirst/-/lodash.upperfirst-4.3.1.tgz#1365edf431480481ef0d1c68957a5ed99d49f7ce"
integrity sha512-sReKOYJIJf74dhJONhU4e0/shzi1trVbSWDOhKYE5XV2O+H7Sb2Dihwuc7xWxVl+DgFPyTqIN3zMfT9cq5iWDg==

lodash@^4.17.15, lodash@^4.17.19, lodash@^4.17.21:
lodash@^4.17.15, lodash@^4.17.19, lodash@^4.17.21, lodash@^4.17.4:
version "4.17.21"
resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.21.tgz#679591c564c3bffaae8454cf0b3df370c3d6911c"
integrity sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg==
Expand Down Expand Up @@ -3824,6 +3837,11 @@ pkg-dir@^4.1.0:
dependencies:
find-up "^4.0.0"

platform@^1.3.3:
version "1.3.6"
resolved "https://registry.yarnpkg.com/platform/-/platform-1.3.6.tgz#48b4ce983164b209c2d45a107adb31f473a6e7a7"
integrity sha512-fnWVljUchTro6RiCFvCXBbNhJc2NijN7oIQxbwsyL0buWJPG85v81ehlHI9fXrJsMNgTofEoWIQeClKpgxFLrg==

prelude-ls@^1.2.1:
version "1.2.1"
resolved "https://registry.yarnpkg.com/prelude-ls/-/prelude-ls-1.2.1.tgz#debc6489d7a6e6b0e7611888cec880337d316396"
Expand Down