Skip to content

Commit

Permalink
[Perf Framework] Fix 0 passed as an option (#17829)
Browse files Browse the repository at this point in the history
* fix 0 passed as an option

* simplification

* changelog

* Update sdk/test-utils/perfstress/CHANGELOG.md
  • Loading branch information
HarshaNalluru authored Sep 24, 2021
1 parent f2f8048 commit 644a4cb
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 6 deletions.
4 changes: 4 additions & 0 deletions sdk/test-utils/perfstress/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## 1.0.0 (Unreleased)

### 2021-09-23

- Bug fix - Running the perf framework with `--<option> 0` does not work correctly as it picks the default value instead. Fixed in [#17829](https://github.com/Azure/azure-sdk-for-js/pull/17829).

### 2021-08-05

- Adds test-proxy tool support to the perf framework. With this, the tests can avoid service throttling by hitting the test-proxy instead to get the recorded responses.
Expand Down
20 changes: 14 additions & 6 deletions sdk/test-utils/perfstress/src/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT license.

import { default as minimist, ParsedArgs as MinimistParsedArgs } from "minimist";
import { isDefined } from "./utils";

/**
* The structure of a PerfStress option. They represent command line parameters.
Expand Down Expand Up @@ -121,19 +122,26 @@ export function parsePerfStressOption<TOptions>(
options: PerfStressOptionDictionary<TOptions>
): Required<PerfStressOptionDictionary<TOptions>> {
const minimistResult: MinimistParsedArgs = minimist(process.argv);
const result = {};
const result: Partial<PerfStressOptionDictionary<TOptions>> = {};

for (const longName of Object.keys(options)) {
// This cast is needed since we're picking up options from process.argv
const option = (options as any)[longName];
const option = options[longName as keyof TOptions];
const { shortName, defaultValue, required } = option;
const value =
minimistResult[longName] || (shortName && minimistResult[shortName]) || defaultValue;
if (required && !value) {
let value: unknown;
if (isDefined(minimistResult[longName])) {
value = minimistResult[longName];
} else if (shortName && isDefined(minimistResult[shortName])) {
value = minimistResult[shortName];
} else {
value = defaultValue;
}

if (required && !isDefined(value)) {
throw new Error(`Option ${longName} is required`);
}
// Options don't need to define longName, it can be derived from the properties PerfStressOptionDictionary.
(result as any)[longName] = {
result[longName as keyof TOptions] = {
...option,
longName,
value
Expand Down
9 changes: 9 additions & 0 deletions sdk/test-utils/perfstress/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,12 @@ export function getHttpClient(url: string): TestProxyHttpClient {
}
return _cachedProxyClients.v2;
}

/**
* Helper TypeGuard that checks if something is defined or not.
* @param thing - Anything
* @internal
*/
export function isDefined<T>(thing: T | undefined | null): thing is T {
return typeof thing !== "undefined" && thing !== null;
}

0 comments on commit 644a4cb

Please sign in to comment.