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

[Unified Recorder] Call proxy-tool through dev-tool #18322

Merged
merged 80 commits into from
Nov 20, 2021
Merged
Show file tree
Hide file tree
Changes from 54 commits
Commits
Show all changes
80 commits
Select commit Hold shift + click to select a range
15ed59e
test-proxy starter code for starting
HarshaNalluru Oct 20, 2021
f5a9992
adding in the requirements
HarshaNalluru Oct 20, 2021
946bf0b
test-proxy starter code for starting
HarshaNalluru Oct 20, 2021
29ad0f3
Merge branch 'main' of https://github.com/Azure/azure-sdk-for-js into…
HarshaNalluru Oct 22, 2021
4bbc37e
clean up
HarshaNalluru Oct 22, 2021
fc25043
gets RootLocation
HarshaNalluru Oct 22, 2021
4f0f2e6
os.platform() === "win32" check
HarshaNalluru Oct 22, 2021
09a20ea
cleanup
HarshaNalluru Oct 22, 2021
e69fa22
testProxyUtils.ts
HarshaNalluru Oct 23, 2021
be70341
checkpoint
HarshaNalluru Oct 26, 2021
0938dc7
node side looks like it's working ✔️
HarshaNalluru Oct 26, 2021
950f46b
readme formatting
HarshaNalluru Oct 26, 2021
fcd64ff
same console.log in win and lin
HarshaNalluru Oct 26, 2021
074bfe4
test:node-with-proxy
HarshaNalluru Oct 27, 2021
5f407f9
Merge branch 'main' of https://github.com/Azure/azure-sdk-for-js into…
HarshaNalluru Oct 27, 2021
ce99f54
"sdk-type": "utility",
HarshaNalluru Oct 27, 2021
cb3342d
lock file
HarshaNalluru Oct 27, 2021
4285651
"sdk-type": "utility",
HarshaNalluru Oct 27, 2021
4adb560
Merge branch 'harshan/issue/18401' of https://github.com/HarshaNallur…
HarshaNalluru Oct 28, 2021
acd56a9
Merge branch 'main' of https://github.com/Azure/azure-sdk-for-js into…
HarshaNalluru Oct 28, 2021
ddd3b7b
lock file
HarshaNalluru Oct 28, 2021
a7147c2
dev-tool test:browser
HarshaNalluru Oct 28, 2021
da195a1
dotenv.config() call not needed since dev-tool does it by default
HarshaNalluru Oct 28, 2021
378a342
run subcommand
HarshaNalluru Oct 28, 2021
803a205
Partly switching to "fs-extra"
HarshaNalluru Oct 28, 2021
91f8a53
fsExtra -> fs
HarshaNalluru Oct 28, 2021
1c46de2
test:node-{js|ts}-input
HarshaNalluru Oct 28, 2021
4469c36
default options
HarshaNalluru Oct 28, 2021
d204966
--single-run
HarshaNalluru Oct 28, 2021
b986e5f
remove dev-tool shortcut
HarshaNalluru Oct 28, 2021
0be5730
runOnlyTestCommand
HarshaNalluru Oct 28, 2021
25968e7
Merge branch 'main' of https://github.com/Azure/azure-sdk-for-js into…
HarshaNalluru Oct 28, 2021
bc48123
dedeuplicate with shouldRunProxyTool and runTestsWithProxyTool methods
HarshaNalluru Oct 28, 2021
f2e1fbf
minor changes
HarshaNalluru Oct 29, 2021
b0cda6a
add console.logs
HarshaNalluru Oct 29, 2021
77e8229
--mocha=\"--whatever\" and refactoring
HarshaNalluru Oct 29, 2021
660ae89
simplify test scripts
HarshaNalluru Oct 29, 2021
a6d1dea
more refactoring
HarshaNalluru Oct 29, 2021
63cdb75
unintended duplication
HarshaNalluru Oct 29, 2021
94e4e0d
const sdkType = contents["sdk-type"];
HarshaNalluru Oct 29, 2021
6ce7ec8
dead code
HarshaNalluru Oct 29, 2021
f6a62c6
removing the if check
HarshaNalluru Oct 29, 2021
41fcb8d
use an array instead
HarshaNalluru Oct 29, 2021
dd0161c
Merge branch 'main' of https://github.com/Azure/azure-sdk-for-js into…
HarshaNalluru Oct 30, 2021
7af4f16
lock file
HarshaNalluru Oct 30, 2021
210ed4d
"sdk-type": "utility",
HarshaNalluru Nov 1, 2021
bed8368
"sdk-type": "utility",
HarshaNalluru Nov 1, 2021
79ddd0f
Merge branch 'harshan/issue/18401' of https://github.com/HarshaNallur…
HarshaNalluru Nov 1, 2021
ad06342
Merge branch 'main' of https://github.com/Azure/azure-sdk-for-js into…
HarshaNalluru Nov 1, 2021
2f1ab3e
lock file
HarshaNalluru Nov 1, 2021
3817355
npm run integration-test:node
HarshaNalluru Nov 1, 2021
c3e44cb
js -> ts
HarshaNalluru Nov 1, 2021
788a308
Merge branch 'main' of https://github.com/Azure/azure-sdk-for-js into…
HarshaNalluru Nov 8, 2021
e4f1ed0
lock file
HarshaNalluru Nov 8, 2021
881b3fc
moving commands/run/testUtils.ts -> src/util/testUtils.ts
HarshaNalluru Nov 8, 2021
baeea7d
lock file
HarshaNalluru Nov 10, 2021
fce7d97
Merge branch 'main' of https://github.com/Azure/azure-sdk-for-js into…
HarshaNalluru Nov 11, 2021
c40196d
lock file
HarshaNalluru Nov 11, 2021
80df60b
bug fix
HarshaNalluru Nov 11, 2021
58fc8d8
PROXY_MANUAL_START
HarshaNalluru Nov 12, 2021
b074e27
getTestMode
HarshaNalluru Nov 16, 2021
348e1c1
readme
HarshaNalluru Nov 16, 2021
8dddf51
lock file from main
HarshaNalluru Nov 16, 2021
6713590
lock file
HarshaNalluru Nov 16, 2021
cb6c383
Merge branch 'main' of https://github.com/Azure/azure-sdk-for-js into…
HarshaNalluru Nov 17, 2021
35a39a3
dump logs
HarshaNalluru Nov 17, 2021
37d151d
fix windows path
HarshaNalluru Nov 17, 2021
e5d4ff0
PROXY_MANUAL_START in karma.conf
HarshaNalluru Nov 17, 2021
a192f99
duplication
HarshaNalluru Nov 17, 2021
5258da3
clean karma conf
HarshaNalluru Nov 17, 2021
a76fcc2
clean package.json
HarshaNalluru Nov 17, 2021
83f9d37
waits for the proxy tool - draft
HarshaNalluru Nov 17, 2021
f124e79
wait-for-proxy-endpoint finish
HarshaNalluru Nov 17, 2021
2283626
Update sdk/test-utils/recorder-new/test/testProxyTests.spec.ts
HarshaNalluru Nov 17, 2021
b790604
beautify the tests
HarshaNalluru Nov 17, 2021
9743b8f
Merge branch 'harshan/issue/17042' of https://github.com/HarshaNallur…
HarshaNalluru Nov 17, 2021
213a315
fix the test mode log
HarshaNalluru Nov 17, 2021
99300d4
minor updates to tests
HarshaNalluru Nov 17, 2021
227e21c
test-info
HarshaNalluru Nov 19, 2021
7696173
no need to pass test mode
HarshaNalluru Nov 19, 2021
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
3,344 changes: 1,759 additions & 1,585 deletions common/config/rush/pnpm-lock.yaml

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions common/tools/dev-tool/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
"private": true,
"prettier": "../eslint-plugin-azure-sdk/prettier.json",
"dependencies": {
"concurrently": "^6.3.0",
"chalk": "~4.1.1",
"dotenv": "^8.2.0",
"fs-extra": "^8.1.0",
Expand All @@ -56,6 +57,7 @@
"@rollup/plugin-json": "^4.0.0",
"@rollup/plugin-multi-entry": "^3.0.0",
"@rollup/plugin-node-resolve": "^8.0.0",
"@types/concurrently": "^6.3.0",
"@types/chai": "^4.1.6",
"@types/chai-as-promised": "^7.1.0",
"@types/fs-extra": "^8.0.0",
Expand Down
4 changes: 3 additions & 1 deletion common/tools/dev-tool/src/commands/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ const log = createPrinter("dev-tool");
export const baseCommands = {
about: () => import("./about"),
package: () => import("./package"),
samples: () => import("./samples")
samples: () => import("./samples"),
"test-proxy": () => import("./test-proxy"),
run: () => import("./run")
} as const;

/**
Expand Down
12 changes: 12 additions & 0 deletions common/tools/dev-tool/src/commands/run/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license

import { subCommand, makeCommandInfo } from "../../framework/command";

export const commandInfo = makeCommandInfo("run", "run scripts such as test:node");
Copy link
Member

Choose a reason for hiding this comment

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

nit: test:node not accurate if the sub commands include both node and browser?


export default subCommand(commandInfo, {
"test:node-ts-input": () => import("./testNodeTSInput"),
"test:node-js-input": () => import("./testNodeJSInput"),
"test:browser": () => import("./testBrowser")
});
24 changes: 24 additions & 0 deletions common/tools/dev-tool/src/commands/run/testBrowser.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license

import { leafCommand, makeCommandInfo } from "../../framework/command";
import { runTestsWithProxyTool } from "./testUtils";

export const commandInfo = makeCommandInfo(
"test:browser",
"runs the browser tests using karma with the default and the provided options; starts the proxy-tool in record and playback modes",
{
karma: {
kind: "string",
description: "Karma options (such as --single-run)",
default: "--single-run"
}
}
);

export default leafCommand(commandInfo, async (options) => {
return runTestsWithProxyTool({
command: `karma start ${options.karma}`,
name: "browser-tests"
});
});
25 changes: 25 additions & 0 deletions common/tools/dev-tool/src/commands/run/testNodeJSInput.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license

import { leafCommand, makeCommandInfo } from "../../framework/command";
import { runTestsWithProxyTool } from "./testUtils";

export const commandInfo = makeCommandInfo(
"test:node-js-input",
"runs the node tests using mocha with the default and the provided options; starts the proxy-tool in record and playback modes",
{
mocha: {
kind: "string",
description:
"Mocha options along with the bundled test file(JS) with rollup as expected by mocha",
default: '--timeout 5000000 "dist-esm/test/{,!(browser)/**/}/*.spec.js"'
}
}
);

export default leafCommand(commandInfo, async (options) => {
return runTestsWithProxyTool({
command: `nyc mocha -r esm --require source-map-support/register --reporter ../../../common/tools/mocha-multi-reporter.js --full-trace ${options.mocha}`,
Copy link
Member

Choose a reason for hiding this comment

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

do we still have packages that bundle the nodejs tests? I hope -r esm still work with those if any exists

Copy link
Member Author

Choose a reason for hiding this comment

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

So, three ways of running the packages

  • js rollup bundle
  • js dist-esm files
  • ts sources

I'll check if test:node-js-input is working for both 1st and 2nd.

name: "node-tests"
});
});
25 changes: 25 additions & 0 deletions common/tools/dev-tool/src/commands/run/testNodeTSInput.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license

import { leafCommand, makeCommandInfo } from "../../framework/command";
import { runTestsWithProxyTool } from "./testUtils";

export const commandInfo = makeCommandInfo(
"test:node-ts-input",
"runs the node tests using mocha with the default and the provided options; starts the proxy-tool in record and playback modes",
{
mocha: {
kind: "string",
description:
"Mocha options along with the test files(glob pattern) in TS as expected by mocha",
default: '--timeout 1200000 --exclude "test/**/browser/*.spec.ts" "test/**/*.spec.ts"'
}
}
);

export default leafCommand(commandInfo, async (options) => {
return runTestsWithProxyTool({
command: `mocha -r esm -r ts-node/register --reporter ../../../common/tools/mocha-multi-reporter.js --full-trace ${options.mocha}`,
name: "node-tests"
});
});
44 changes: 44 additions & 0 deletions common/tools/dev-tool/src/commands/run/testUtils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import { isProxyToolActive } from "../../util/testProxyUtils";
witemple-msft marked this conversation as resolved.
Show resolved Hide resolved
import concurrently from "concurrently";
import { createPrinter } from "../../util/printer";

const log = createPrinter("preparing-proxy-tool");

async function shouldRunProxyTool(): Promise<boolean> {
const mode = process.env.TEST_MODE;
if (mode === "live") {
return false; // No need to start the proxy tool in the live mode
} else {
try {
await isProxyToolActive();
// No need to run a new one if it is already active
// Especially, CI uses this path
log.info(
`Proxy tool seems to be active, not attempting to start the test proxy at http://localhost:5000 & https://localhost:5001.\n`
);
return false;
} catch (error) {
if ((error as { code: string }).code === "ECONNREFUSED") {
// Proxy tool is not active, attempt to start the proxy tool now
return true;
} else {
throw error;
}
}
}
}

export async function runTestsWithProxyTool(testCommandObj: concurrently.CommandObj) {
const testProxyCMD = "dev-tool test-proxy start";
if (
await shouldRunProxyTool() // Boolean to figure out if we need to run just the mocha command or the test-proxy too
) {
await concurrently([{ command: testProxyCMD }, testCommandObj], {
killOthers: ["failure", "success"],
successCondition: "first"
});
} else {
await concurrently([testCommandObj]);
}
return true;
}
13 changes: 13 additions & 0 deletions common/tools/dev-tool/src/commands/test-proxy/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license

import { subCommand, makeCommandInfo } from "../../framework/command";

export const commandInfo = makeCommandInfo(
"test-proxy",
"runs the proxy-tool with the `docker run ...` command"
);

export default subCommand(commandInfo, {
start: () => import("./start")
});
18 changes: 18 additions & 0 deletions common/tools/dev-tool/src/commands/test-proxy/start.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

import { leafCommand, makeCommandInfo } from "../../framework/command";
import { config } from "dotenv";
import { startProxyTool } from "../../util/testProxyUtils";
config();

export const commandInfo = makeCommandInfo(
"test-proxy",
"runs the proxy-tool with the `docker run ...` command",
{}
);

export default leafCommand(commandInfo, async (_options) => {
await startProxyTool(undefined);
return true;
});
83 changes: 83 additions & 0 deletions common/tools/dev-tool/src/util/testProxyUtils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

import { spawn } from "child_process";
import path from "path";
import { IncomingMessage, request, RequestOptions } from "http";
Copy link
Member

Choose a reason for hiding this comment

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

I'm in favor of using a simpler HTTP client in dev-tool such as axios.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is the only HTTP request that is made in the whole of dev-tool.
I don't want to take a dependency on another package if this serves my purpose.

Copy link
Member

Choose a reason for hiding this comment

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

Very well, When/if we have more HTTP usage I'd like to replace it with axios, node-fetch, etc. (something a little bit more "batteries-included").

Copy link
Contributor

Choose a reason for hiding this comment

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

core-rest-pipelines should help 🤔 at least in the future

Copy link
Member Author

Choose a reason for hiding this comment

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

That'll add circular dependencies @sadasant.
core packages depend on dev-tool

Copy link
Member

Choose a reason for hiding this comment

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

I don't think we want to use core for incidental stuff in dev-tool. It's designed for the client libraries and unless we really have some kind of request pipeline in dev-tool where performance and consistent configurability with the clients is important, I think it'll be nicer to just use a simple API.

Copy link
Contributor

Choose a reason for hiding this comment

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

That'll add circular dependencies @sadasant.

Not if you pin a version 🤔 since dev-tools is only a devDependency. But I agree with Will 👍

import fs from "fs-extra";
import { createPrinter } from "./printer";

const log = createPrinter("test-proxy");
export async function startProxyTool(mode: string | undefined) {
log.info(`===TEST_MODE="${mode}"===`);
log.info(
`Attempting to start test proxy at http://localhost:5000 & https://localhost:5001.\n`
Copy link
Contributor

Choose a reason for hiding this comment

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

Can these ports be parametrized?

Copy link
Member Author

Choose a reason for hiding this comment

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

Logged an issue for this at #15829, will tackle in a future PR.

);

const subprocess = spawn(await getDockerRunCommand(), [], {
shell: true
});

const outFileName = "test-proxy-output.log";
const out = fs.createWriteStream(`./${outFileName}`,{flags:'a'});
subprocess.stdout.pipe(out);
subprocess.stderr.pipe(out);

log.info(`Check the output file "${outFileName}" for test-proxy logs.`);
}

async function getRootLocation(start?: string): Promise<string> {
start ??= process.cwd();
if (await fs.pathExists(path.join(start, "rush.json"))) {
return start;
} else {
const nextPath = path.resolve(start, "..");
if (nextPath === start) {
throw new Error("Reached filesystem root, but no rush.json was found.");
} else {
return getRootLocation(nextPath);
}
}
}

async function getDockerRunCommand() {
const repoRoot = await getRootLocation(); // /workspaces/azure-sdk-for-js/
const testProxyRecordingsLocation = "/etc/testproxy";
Copy link
Member

Choose a reason for hiding this comment

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

nit: /etc is usually for system configurations. maybe use another directory?

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Azure/azure-sdk-tools#2279

@HarshaNalluru the PR is submitted to sync the proxy tool tag update, I will need to change this script as well. That will keep everything in sync.

const allowLocalhostAccess = "--add-host host.docker.internal:host-gateway";
const imageToLoad = `azsdkengsys.azurecr.io/engsys/testproxy-lin:${await getImageTag()}`;
return `docker run -v ${repoRoot}:${testProxyRecordingsLocation} -p 5001:5001 -p 5000:5000 ${allowLocalhostAccess} ${imageToLoad}`;
HarshaNalluru marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

Can these ports be parametrized?

Copy link
Member Author

Choose a reason for hiding this comment

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

Logged an issue for this at #15829, will tackle in a future PR.

}

export async function isProxyToolActive() {
await makeRequest("http://localhost:5000/info/available", {});
log.info(`Proxy tool seems to be active at http://localhost:5000\n`);
Copy link
Contributor

Choose a reason for hiding this comment

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

Same parametrized question :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Logged an issue for this at #15829, will tackle in a future PR.

}

async function makeRequest(uri: string, requestOptions: RequestOptions): Promise<IncomingMessage> {
return new Promise<IncomingMessage>((resolve, reject) => {
const req = request(uri, requestOptions, resolve);
req.once("error", reject);
req.end();
});
}

async function getImageTag() {
// Grab the tag from the `/eng/common/testproxy/docker-start-proxy.ps1` file [..is used to run the proxy-tool in the CI]
//
// $SELECTED_IMAGE_TAG = "1147815";
// (Bot regularly updates the tag in the file above.)
try {
const contentInPWSHScript = await fs.readFile(
`${path.join(await getRootLocation(), "eng/common/testproxy/docker-start-proxy.ps1")}`,
Copy link
Contributor

Choose a reason for hiding this comment

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

can this path be parametrized? In case someone wants to change it

Copy link
Member Author

Choose a reason for hiding this comment

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

Why would someone not want the root location?

Copy link
Member Author

Choose a reason for hiding this comment

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

This path is to find the docker-start-proxy.ps1 script.
Which can only be found if you start at the root of the repo. :)

Copy link
Contributor

Choose a reason for hiding this comment

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

oh ok ok. Gotcha! Thanks!

"utf-8"
);
const tag = contentInPWSHScript.match(/\$SELECTED_IMAGE_TAG \= \"(.*)\"/)![1];
log.info(`Image tag obtained from the powershell script => ${tag}\n`);
return tag;
} catch (_) {
log.warn(
`Unable to get the image tag from the powershell script, trying "latest" tag instead\n`
);
return "latest";
}
}
14 changes: 7 additions & 7 deletions sdk/test-utils/recorder-new/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,20 +21,20 @@
"clean": "rimraf dist dist-esm test-dist typings *.tgz *.log",
"extract-api": "echo skipped",
"format": "prettier --write --config ../../../.prettierrc.json --ignore-path ../../../.prettierignore \"src/**/*.ts\" \"test/**/*.ts\" \"*.{js,json}\"",
"integration-test:browser": "echo skipped",
"integration-test:node": "echo skipped",
"integration-test:browser": "concurrently \"npm run tests:server\" \"npm run test:browser-with-proxy\" --kill-others --success first",
"integration-test:node": "concurrently \"npm run tests:server\" \"npm run test:node-with-proxy\" --kill-others --success first",
"test:node-with-proxy": "dev-tool run test:node-ts-input --mocha=\"--timeout 1200000 'test/*.spec.ts'\"",
"test:browser-with-proxy": "dev-tool run test:browser",
"integration-test": "npm run integration-test:node && npm run integration-test:browser",
"tests:server": "cross-env TS_NODE_COMPILER_OPTIONS=\"{\\\"module\\\": \\\"commonjs\\\"}\" ts-node test/utils/server.ts",
"temp-integration-test:browser": "concurrently \"npm run tests:server\" \"karma start --single-run\" --kill-others --success first",
"temp-integration-test:node": "concurrently \"npm run tests:server\" \"nyc mocha -r esm --require ts-node/register --reporter ../../../common/tools/mocha-multi-reporter.js --timeout 5000000 --full-trace 'test/*.spec.ts'\" --kill-others --success first",
"lint:fix": "eslint package.json src test --ext .ts --fix --fix-type [problem,suggestion]",
"lint": "eslint package.json src test --ext .ts -f html -o recorder-lintReport.html || exit 0",
"pack": "npm pack 2>&1",
"unit-test:browser": "echo skipped",
"unit-test:node": "echo skipped",
"unit-test:browser": "npm run integration-test:browser",
"unit-test:node": "npm run integration-test:node",
"unit-test": "npm run unit-test:node && npm run unit-test:browser",
"test:browser": "npm run clean && npm run build && npm run temp-integration-test:browser",
"test:node": "npm run clean && npm run build:test && npm run temp-integration-test:node",
"test:node": "npm run clean && npm run build:test && npm run integration-test:node",
"test": "npm run clean && npm run build:test && npm run unit-test",
"docs": "echo Skipped."
},
Expand Down
1 change: 1 addition & 0 deletions sdk/test-utils/testing-recorder-new/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,4 @@ You will need to create a local `.env` file under the same directory as this rea
"STORAGE_CONNECTION_STRING",
"STORAGE_SAS_URL",
"TABLES_SAS_CONNECTION_STRING"
```
8 changes: 4 additions & 4 deletions sdk/test-utils/testing-recorder-new/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,14 @@
"clean": "rimraf dist dist-esm test-dist types *.tgz",
"extract-api": "echo skipped",
"format": "prettier --write --config ../../../.prettierrc.json --ignore-path ../../../.prettierignore \"src/**/*.ts\" \"test/**/*.ts\" \"*.{js,json}\"",
"integration-test:browser": "karma start --single-run",
"integration-test:node": "mocha -r esm --require ts-node/register --reporter ../../../common/tools/mocha-multi-reporter.js --timeout 1200000 --full-trace \"test/*.spec.ts\"",
"integration-test:browser": "dev-tool run test:browser",
"integration-test:node": "dev-tool run test:node-ts-input --mocha=\"--timeout 1200000 'test/*.spec.ts'\"",
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to have a separate command without timeout? I have found it useful in the past, I wonder if it would be useful here, or in general (outside of this line I’m pinning with this comment), to possibly run the new recorder “without timeouts”.

Copy link
Member Author

Choose a reason for hiding this comment

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

Unrelated to this PR. It would be a broader discussion irrespective of the dev-tool extension that this PR does.

If you want the timeout, call as "dev-tool run test:node-ts-input --mocha=\"--timeout 1200000 'test/*.spec.ts'\"".
If you don't want the timeout, call "dev-tool run test:node-ts-input --mocha=\"'test/*.spec.ts'\"" or just "dev-tool run test:node-ts-input.

The same applies to any of the options that you'd pass to the mocha command.

Copy link
Contributor

Choose a reason for hiding this comment

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

of course, of course! Thanks for following up! Approving

"integration-test": "npm run integration-test:node && npm run integration-test:browser",
"lint:fix": "eslint package.json src test --ext .ts --fix --fix-type [problem,suggestion]",
"lint": "eslint package.json src test --ext .ts -f html -o recorder-lintReport.html || exit 0",
"pack": "npm pack 2>&1",
"unit-test:browser": "echo skipped",
"unit-test:node": "echo skipped",
"unit-test:browser": "npm run integration-test:browser",
"unit-test:node": "npm run integration-test:node",
"unit-test": "npm run unit-test:node && npm run unit-test:browser",
"test:browser": "npm run clean && npm run build && npm run integration-test:browser",
"test:node": "npm run clean && npm run build:test && npm run integration-test:node",
Expand Down