Skip to content

Commit e8766b7

Browse files
authored
[Eng] Update Dependency Test & Docs (Azure#34245)
After the unifying test changes in this PR Azure#34132, this PR aims to: * Remove references to `unit-test` and `integration-test` with `test` * Update min/max testing to look for the right updated scripts
1 parent eae9d34 commit e8766b7

File tree

9 files changed

+37
-101
lines changed

9 files changed

+37
-101
lines changed

.vscode/launch.json

Lines changed: 2 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -15,56 +15,17 @@
1515
"cwd": "${workspaceFolder}/sdk/${input:package-directory}"
1616
},
1717
{
18-
"name": "unit:current-file",
18+
"name": "test:current-file",
1919
"type": "node",
2020
"request": "launch",
2121
"runtimeExecutable": "rushx",
22-
"runtimeArgs": ["unit-test:node"],
22+
"runtimeArgs": ["test:node"],
2323
"args": ["--", "--inspect-brk", "--no-file-parallelism", "${fileBasename}"],
2424
"autoAttachChildProcesses": false,
2525
"skipFiles": ["<node_internals>/**"],
2626
"console": "integratedTerminal",
2727
"attachSimplePort": 9229,
2828
"cwd": "${fileDirname}"
29-
},
30-
{
31-
"name": "unit-customized:current-file",
32-
"type": "node",
33-
"request": "launch",
34-
"runtimeExecutable": "rushx",
35-
"runtimeArgs": ["unit-test:node"],
36-
"args": ["--inspect-brk", "--no-file-parallelism", "${fileBasename}"],
37-
"autoAttachChildProcesses": false,
38-
"skipFiles": ["<node_internals>/**"],
39-
"console": "integratedTerminal",
40-
"attachSimplePort": 9229,
41-
"cwd": "${fileDirname}"
42-
},
43-
{
44-
"name": "integration:current-file",
45-
"type": "node",
46-
"request": "launch",
47-
"runtimeExecutable": "rushx",
48-
"runtimeArgs": ["integration-test:node"],
49-
"args": ["--", "--inspect-brk", "--no-file-parallelism", "${fileBasename}"],
50-
"autoAttachChildProcesses": false,
51-
"skipFiles": ["<node_internals>/**"],
52-
"console": "integratedTerminal",
53-
"attachSimplePort": 9229,
54-
"cwd": "${fileDirname}"
55-
},
56-
{
57-
"name": "integration-customized:current-file",
58-
"type": "node",
59-
"request": "launch",
60-
"runtimeExecutable": "rushx",
61-
"runtimeArgs": ["integration-test:node"],
62-
"args": ["--inspect-brk", "--no-file-parallelism", "${fileBasename}"],
63-
"autoAttachChildProcesses": false,
64-
"skipFiles": ["<node_internals>/**"],
65-
"console": "integratedTerminal",
66-
"attachSimplePort": 9229,
67-
"cwd": "${fileDirname}"
6829
}
6930
],
7031
"inputs": [

CONTRIBUTING.md

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ If you encounter any bugs with the library, please file an issue in the [Issues]
2929

3030
Some guidance for when you make a contribution:
3131

32-
- Add/update unit tests and code as required by your change
33-
- Make sure you run all the unit tests on the affected platform(s)/languages. If the change is in common code, generally running on one platform would be acceptable.
32+
- Add/update tests and code as required by your change
33+
- Make sure you run all the tests on the affected platform(s)/languages. If the change is in common code, generally running on one platform would be acceptable.
3434
- Run end-to-end tests or simple sample code to make sure the lib works in an end-to-end scenario.
3535

3636
## Big contributions
@@ -199,18 +199,12 @@ All projects have at least the following scripts:
199199
- `clean`: Remove generated and temporary files
200200
- `execute:samples`: Execute samples using the source code
201201
- `format`: Reformat project files with Prettier
202-
- `integration-test:browser`: Execute browser integration tests
203-
- `integration-test:node`: Execute Node integration tests
204-
- `integration-test`: Execute all integration tests
205202
- `lint:fix`: Fix ESLint issues within the project
206203
- `lint`: Show ESLint issues within the project
207204
- `pack`: Run `npm pack` on the project
208205
- `test:browser`: Execute browser dev tests
209206
- `test:node`: Execute Node dev tests
210207
- `test`: Execute all dev tests
211-
- `unit-test:browser`: Execute browser unit tests
212-
- `unit-test:node`: Execute Node unit tests
213-
- `unit-test`: Execute all unit tests
214208

215209
Projects may optionally have the following scripts:
216210

@@ -290,7 +284,7 @@ The second type of libraries is more complex to develop and maintain because the
290284

291285
Rush assumes that anything printed to `STDERR` is a warning. Your package scripts should avoid writing to `STDERR` unless emitting warnings or errors, since this will cause Rush to flag them as warnings during the execution of your build or script command. If your library uses a tool that can't be configured this way, you can still append `2>&1` to the command which will redirect all output to `STDOUT`. You won't see warnings show up, but Rush will still consider the command to have failed as long as it returns a nonzero exit code.
292286

293-
In general, it's recommended to avoid using NPM [hook scripts](https://docs.npmjs.com/misc/scripts) (those starting with `pre` / `post`). The build system will always explicitly run the `install`, `build`, `build:test`, `pack`, `lint`, `unit-test`, and `integration-test` scripts at the appropriate times during the build. Adding hooks that perform steps like installing dependencies or compiling the source code will at best slow down the build, and at worst may lead to difficult to diagnose build failures.
287+
In general, it's recommended to avoid using NPM [hook scripts](https://docs.npmjs.com/misc/scripts) (those starting with `pre` / `post`). The build system will always explicitly run the `install`, `build`, `build:test`, `pack`, `lint`, and `test` scripts at the appropriate times during the build. Adding hooks that perform steps like installing dependencies or compiling the source code will at best slow down the build, and at worst may lead to difficult to diagnose build failures.
294288

295289
Because Rush uses PNPM to download and manage dependencies, it's **_especially_** important to make sure that none of your package scripts are calling `npm install` when your library is built via the Rush toolchain. Most commonly this occurs in a `prepack` or `prebuild` script. Ensure your library does not contain these scripts - or if you determine that such a script is required, ensure that it doesn't run `npm install`.
296290

common/config/rush/command-line.json

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -47,28 +47,6 @@
4747
"summary": "Reformat projects",
4848
"enableParallelism": true
4949
},
50-
{
51-
"commandKind": "bulk",
52-
"name": "integration-test:browser",
53-
"summary": "Execute browser integration tests defined in projects",
54-
"enableParallelism": true,
55-
"allowWarningsInSuccessfulBuild": true
56-
},
57-
{
58-
"commandKind": "bulk",
59-
"name": "integration-test:node",
60-
"summary": "Execute Node integration tests defined in projects",
61-
"enableParallelism": true,
62-
"allowWarningsInSuccessfulBuild": true
63-
},
64-
{
65-
"commandKind": "bulk",
66-
"name": "integration-test",
67-
"summary": "Execute integration tests defined in projects",
68-
"description": "Execute tests which require an actual service instance (live Azure, emulator, etc). Integration tests typically require service endpoints to be specified via environment variables in order to run.",
69-
"enableParallelism": true,
70-
"allowWarningsInSuccessfulBuild": true
71-
},
7250
{
7351
"commandKind": "bulk",
7452
"name": "lint",

common/tools/dev-tool/src/commands/admin/list/packages.ts

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,10 @@ export default async function IdentifyPackage(
3939
const fullProjectPath = path.join(root, projectFolder);
4040
const { packageJson } = await resolveProject(fullProjectPath);
4141
42-
const unitTestNodeScript = packageJson.scripts["unit-test:node"];
43-
const integrationTestNodeScript = packageJson.scripts["integration-test:node"];
42+
const testNodeScript = packageJson.scripts["test:node"];
4443
45-
if (unitTestNodeScript.includes("dev-tool run test:node-js-input") ||
46-
integrationTestNodeScript.includes("dev-tool run test:node-js-input") ||
47-
unitTestNodeScript.includes("dev-tool run test:node-tsx-ts") ||
48-
integrationTestNodeScript.includes("dev-tool run test:node-tsx-ts")) {
44+
if (testNodeScript.includes("dev-tool run test:node-js-input") ||
45+
testNodeScript.includes("dev-tool run test:node-tsx-ts")) {
4946
if (!packageJson.devDependencies["tsx"]) {
5047
console.log(` updating ${packageName} ${fullProjectPath} testing scripts`);
5148
packageJson.devDependencies["tsx"] = "^4.7.1";

common/tools/dev-tool/src/commands/admin/migrate-package.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -348,15 +348,15 @@ function setScriptsSection(
348348
scripts["build"] = "npm run clean && dev-tool run build-package && dev-tool run extract-api";
349349

350350
if (options.browser) {
351-
scripts["unit-test:browser"] =
351+
scripts["test:browser"] =
352352
"npm run clean && dev-tool run build-package && dev-tool run build-test && dev-tool run test:vitest --browser";
353353
}
354354

355-
scripts["unit-test:node"] = "dev-tool run test:vitest";
355+
scripts["test:node"] = "dev-tool run test:vitest";
356+
scripts["test:node:esm"] = "dev-tool run test:vitest --esm";
356357

357358
if (options.isArm) {
358-
scripts["unit-test:browser"] = "echo skipped";
359-
scripts["integration-test:node"] = "dev-tool run test:vitest --esm";
359+
scripts["test:browser"] = "echo skipped";
360360
}
361361

362362
if (!options.formatTests) {

eng/tools/dependency-testing/README.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,16 +75,21 @@ Note : If the above step fails, you can reset the repo: `git clean -f -x -d` (Wa
7575
### Troubleshooting guide for min-max test failures
7676

7777
#### This is not a module
78+
7879
When you see an error message as shown below, it implies that there has been an internal source reference in a file inside the test/public folder that has been replaced by an empty file to avoid multiple versions of a dependency being pulled in.
7980

8081
```
8182
communicationIdentityClient.mocked.spec.ts(8,68): error TS2306: File '/mnt/vss/_work/1/s/sdk/communication/communication-identity/test/public/utils/mockHttpClients.ts' is not a module.
8283
utils/testCommunicationIdentityClient.ts(17,8): error TS2306: File '/mnt/vss/_work/1/s/sdk/communication/communication-identity/test/public/utils/mockHttpClients.ts' is not a module.
8384
```
85+
8486
To fix the error, first locate where in the above file are you using the internal source reference. For example, from the above error, you notice that `/mnt/vss/_work/1/s/sdk/communication/communication-identity/test/public/utils/mockHttpClients.ts` probably has an internal source reference, which is why it's not accessible anymore since it's replaced by an empty file by the dependency-testing tool. This is the internal source reference detected in the `test/public/utils/mockHttpClients.ts` file.
87+
8588
```
8689
import { CommunicationIdentityAccessTokenResult } from "../../../src/generated/src/models";
8790
```
91+
8892
After locating the internal source reference, you have one of the following options:
93+
8994
- Either expose the above interface/ constant through the public API in the src/index.ts file of the Azure-SDK and change the import to accessing it from the src in this format - `../../../src` so that it becomes a public reference
9095
- Or move the tests and references to test/internal folder if there's something you are accessing that cannot be exposed publicly.

eng/tools/dependency-testing/index.js

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -56,34 +56,35 @@ function outputTestPath(projectFolderPath, sourceDir, testFolder) {
5656
}
5757
/**
5858
* This function uses the package's timeout in it's package.json for
59-
* the integration-test:node command for the min-max tests.
59+
* the test:node command for the min-max tests.
6060
* This function basically does a string search for "timeout" / "test-timeout" / "hook-timeout" in the package's package.json
6161
* and replaces the command for timeout in new package.json in the test or test/public folder.
6262
* @param testPackageJson - the package.json that will be created in the test folder
6363
* @param packageJsonContents - the package's package.json contents
6464
*/
6565
async function usePackageTestTimeout(testPackageJson, packageJsonContents) {
66-
if (packageJsonContents.scripts["integration-test:node"]) {
66+
if (packageJsonContents.scripts["test:node"]) {
6767
// Replace any test-timeout
6868
let timeoutPattern = /--(test-)?timeout\s+(\d+)/;
69-
let replaceWithTimeout =
70-
packageJsonContents.scripts["integration-test:node"].match(timeoutPattern);
69+
let replaceWithTimeout = packageJsonContents.scripts["test:node"].match(timeoutPattern);
7170
if (replaceWithTimeout !== null) {
7271
const timeoutArgument = `${replaceWithTimeout[1] || ""}timeout`;
7372
const packageTimeout = replaceWithTimeout[2];
74-
testPackageJson.scripts["integration-test:node"] = testPackageJson.scripts[
75-
"integration-test:node"
76-
].replace(timeoutPattern, `--${timeoutArgument} ${packageTimeout}`);
73+
testPackageJson.scripts["test:node"] = testPackageJson.scripts["test:node"].replace(
74+
timeoutPattern,
75+
`--${timeoutArgument} ${packageTimeout}`,
76+
);
7777
}
7878

7979
// Replace any hook-timeout
8080
timeoutPattern = /--hook-timeout\s+(\d+)/; // this is only a vitest concept, so there's just one pattern
81-
replaceWithTimeout = packageJsonContents.scripts["integration-test:node"].match(timeoutPattern);
81+
replaceWithTimeout = packageJsonContents.scripts["test:node"].match(timeoutPattern);
8282
if (replaceWithTimeout !== null) {
8383
const packageTimeout = replaceWithTimeout[1];
84-
testPackageJson.scripts["integration-test:node"] = testPackageJson.scripts[
85-
"integration-test:node"
86-
].replace(timeoutPattern, `--hook-timeout ${packageTimeout}`);
84+
testPackageJson.scripts["test:node"] = testPackageJson.scripts["test:node"].replace(
85+
timeoutPattern,
86+
`--hook-timeout ${packageTimeout}`,
87+
);
8788
}
8889
}
8990
}
@@ -117,10 +118,10 @@ async function insertPackageJson(
117118
packageJsonContents.name.replace("@azure-rest/", "azure-rest-") + "-test";
118119
}
119120
testPackageJson.type = packageJsonContents.type;
120-
if (packageJsonContents.scripts["integration-test:node"].includes("vitest")) {
121-
testPackageJson.scripts["integration-test:node"] =
121+
if (packageJsonContents.scripts["test:node"].includes("vitest")) {
122+
testPackageJson.scripts["test:node"] =
122123
"dev-tool run test:vitest -- -c vitest.dependency-test.config.ts --test-timeout 180000 --hook-timeout 180000";
123-
testPackageJson.scripts["integration-test:browser"] =
124+
testPackageJson.scripts["test:browser"] =
124125
"dev-tool run build-test && dev-tool run test:vitest --browser -- -c vitest.dependency-test.browser.config.ts";
125126
testPackageJson.scripts["build"] = "echo skipped.";
126127
}
@@ -419,7 +420,7 @@ async function main(argv) {
419420
testFolder,
420421
);
421422
await insertTsConfigJson(targetPackagePath, testFolder);
422-
if (packageJsonContents.scripts["integration-test:node"].includes("vitest")) {
423+
if (packageJsonContents.scripts["test:node"].includes("vitest")) {
423424
copyVitestConfig(targetPackagePath, testFolder);
424425
}
425426
if (dryRun) {

eng/tools/dependency-testing/templates/package.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,9 @@
88
},
99
"scripts": {
1010
"build": "tsc -p .",
11-
"integration-test:browser": "karma start --single-run",
12-
"integration-test:node": "mocha --require tsx --require source-map-support/register --reporter mocha-multi-reporter.js --reporter-option output=test-results.xml --timeout 350000 --full-trace \"dist-esm/**/{,!(browser)/**/}*.spec.js\" --exit",
13-
"integration-test": "npm run integration-test:node && echo skipped: npm run integration-test:browser"
11+
"test:browser": "karma start --single-run",
12+
"test:node": "mocha --require tsx --require source-map-support/register --reporter mocha-multi-reporter.js --reporter-option output=test-results.xml --timeout 350000 --full-trace \"dist-esm/**/{,!(browser)/**/}*.spec.js\" --exit",
13+
"test": "npm run test:node && echo skipped: npm run test:browser"
1414
},
1515
"repository": {
1616
"type": "git",

sdk/test-utils/recorder/ASSET_SYNC_WORKFLOW.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ At this point, you should have an `assets.json` file under your SDK.
5656

5757
Run your tests using the usual [package.json scripts].
5858

59-
`rushx integration-test:node`, for example.
59+
`rushx test:node`, for example.
6060

6161
With asset sync enabled, there is one extra step that must be taken before you create a PR with changes to recorded tests: you must push the new recordings to the assets repo. This is done with the following command:
6262

0 commit comments

Comments
 (0)