Skip to content

Commit

Permalink
[Unified Recorder] TestProxyClient takes test context to generate rec…
Browse files Browse the repository at this point in the history
…ordings at desired location (#17388)

* TestProxyClient takes test context to generate recordings at desired location

* set /workspaces/azure-sdk-for-js/ as entry point

* use Test as testContext type

* update recorder-new tests

* update tests with before and after each

* relativePathCalculator - for browser => done

* create utils folder

* export relativeRecordingsPathForBrowser to use in karma.conf

* utils folder updates

* RECORDINGS_RELATIVE_PATH env variable in karma.conf

* node side draft

* relativeRecordingsPathForNode

* findRecordingsFolderPath

* remove console.logs

* Update sdk/test-utils/recorder-new/README.md

* throw new Error(
          "Unable to determine the recording file path, testContext provided is not defined."
        );

* Update sdk/test-utils/recorder-new/README.md

* lock file

* --net=host docs

* sample recordings

* refactor existing tests

* server and tests hitting the server

* unrelated changes in package.json

* changelog

* lock file

* Jeremy's feedbackl

* lock file

* lock file

* Update sdk/test-utils/recorder-new/README.md

Co-authored-by: Will Temple <witemple@microsoft.com>

* Update sdk/test-utils/recorder-new/README.md

Co-authored-by: Will Temple <witemple@microsoft.com>

* readme

* removing the exclamations. 🙂

* address feedback

* clientHttpClient

* sample recordings

* relativeRecordingsPathForBrowser -> relativeRecordingsPath

* massive update to relativeRecordingsPath - which works fine on linux

* RECORDINGS_RELATIVE_PATH in karma.conf

* dist-esm/test/index.spec.js doesn't exist

* lock file

* updates for windows

* `--add-host host.docker.internal:host-gateway`

* more comments

* remove unintended file

* Update sdk/test-utils/recorder-new/CHANGELOG.md

Co-authored-by: Daniel Rodríguez <sadasant@users.noreply.github.com>

* remove sq brackets

Co-authored-by: Will Temple <witemple@microsoft.com>
Co-authored-by: Daniel Rodríguez <sadasant@users.noreply.github.com>
  • Loading branch information
3 people authored Oct 4, 2021
1 parent 0e569b5 commit e099b12
Show file tree
Hide file tree
Showing 25 changed files with 1,497 additions and 902 deletions.
1,879 changes: 1,073 additions & 806 deletions common/config/rush/pnpm-lock.yaml

Large diffs are not rendered by default.

14 changes: 14 additions & 0 deletions sdk/test-utils/recorder-new/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# Release History

## 1.0.0 (Unreleased)

## 2021-09-27

- `TestProxyClient` now takes the test context to determine the location of the recordings. [#17388](https://github.com/Azure/azure-sdk-for-js/pull/17388)
- Adds a server for the tests, to play the role of an actual service to be able to test the proxy-tool end-to-end.
[#17388](https://github.com/Azure/azure-sdk-for-js/pull/17388)

## 2021-07-17

- Building the unified recorder prototype leveraging the proxy-tool, works for both core-v1 and core-v2 SDKs. Shows data-tables and storage-queue as examples for core-v2 and core-v1 respectively.
[#15826](https://github.com/Azure/azure-sdk-for-js/pull/15826)
7 changes: 5 additions & 2 deletions sdk/test-utils/recorder-new/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,14 @@ Feature work is being tracked at [#15829](https://github.com/Azure/azure-sdk-for

Run this command

> `docker run -v temp-location:/etc/testproxy -p 5001:5001 -p 5000:5000 azsdkengsys.azurecr.io/engsys/testproxy-lin:latest`
> `docker run -v /workspaces/azure-sdk-for-js/:/etc/testproxy -p 5001:5001 -p 5000:5000 azsdkengsys.azurecr.io/engsys/testproxy-lin:latest`
Map the root directory of the azure-sdk-for-js repo to `/etc/testproxy` inside the container for an accurate location while generating recordings.

(Eventually, recorder will trigger this for you!)

[Note: Update `temp-location` in the command to your desired location.]
Add `--add-host host.docker.internal:host-gateway` for linux to access host's network(to access `localhost`) through `host.docker.internal`.
Docker for Windows and Mac support `host.docker.internal` as a functioning alias for localhost.

If the above command doesn't work directly, try [Troubleshooting Access to Public Container Registry](https://github.com/Azure/azure-sdk-tools/tree/main/tools/test-proxy/docker#troubleshooting-access-to-public-container-registry).

Expand Down
5 changes: 4 additions & 1 deletion sdk/test-utils/recorder-new/karma.conf.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
// https://github.com/karma-runner/karma-chrome-launcher
const { relativeRecordingsPath } = require("./dist/index.js");
process.env.CHROME_BIN = require("puppeteer").executablePath();
require("dotenv").config({ path: "../.env" });

process.env.RECORDINGS_RELATIVE_PATH = relativeRecordingsPath();

module.exports = function(config) {
config.set({
// base path that will be used to resolve all patterns (eg. files, exclude)
Expand Down Expand Up @@ -46,7 +49,7 @@ module.exports = function(config) {
// inject following environment values into browser testing with window.__env__
// environment values MUST be exported or set with same console running "karma start"
// https://www.npmjs.com/package/karma-env-preprocessor
// envPreprocessor: [],
envPreprocessor: ["RECORDINGS_RELATIVE_PATH"],

// test results reporter to use
// possible values: 'dots', 'progress'
Expand Down
24 changes: 19 additions & 5 deletions sdk/test-utils/recorder-new/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@
"main": "dist/index.js",
"module": "dist-esm/src/index.js",
"types": "./types/src/index.d.ts",
"browser": {
"./dist-esm/src/utils/relativePathCalculator.js": "./dist-esm/src/utils/relativePathCalculator.browser.js",
"./dist-esm/test/utils/server.js": "./dist-esm/test/utils/server.browser.js"
},
"scripts": {
"audit": "node ../../../common/scripts/rush-audit.js && rimraf node_modules package-lock.json && npm i --package-lock-only 2>&1 && npm audit",
"build:browser": "echo skipped",
Expand All @@ -17,14 +21,17 @@
"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": "karma start --single-run",
"integration-test:node": "nyc mocha -r esm --require source-map-support/register --reporter ../../../common/tools/mocha-multi-reporter.js --timeout 5000000 --full-trace dist-esm/test/*.spec.js dist-esm/test/node/*.spec.js",
"integration-test:browser": "echo skipped",
"integration-test:node": "echo skipped",
"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": "karma start --single-run",
"unit-test:node": "mocha -r esm --require ts-node/register --reporter ../../../common/tools/mocha-multi-reporter.js --timeout 1200000 --full-trace \"test/*.spec.ts\"",
"unit-test:browser": "echo skipped",
"unit-test:node": "echo skipped",
"unit-test": "npm run unit-test:node && npm run unit-test:browser",
"test:browser": "npm run clean && npm run build:test && npm run unit-test:browser",
"test:node": "npm run clean && npm run build:test && npm run unit-test:node",
Expand Down Expand Up @@ -59,15 +66,18 @@
"dependencies": {
"@azure-tools/test-recorder": "^1.0.0",
"@azure/core-http": "^2.0.0",
"@azure/core-rest-pipeline": "^1.1.0"
"@azure/core-rest-pipeline": "^1.1.0",
"@azure/test-utils": "^1.0.0"
},
"devDependencies": {
"@azure/core-client": "^1.0.0",
"@azure/dev-tool": "^1.0.0",
"@azure/eslint-plugin-azure-sdk": "^3.0.0",
"@rollup/plugin-commonjs": "11.0.2",
"@rollup/plugin-multi-entry": "^3.0.0",
"@rollup/plugin-node-resolve": "^8.0.0",
"@rollup/plugin-replace": "^2.2.0",
"@types/express": "^4.16.0",
"@types/fs-extra": "^8.0.0",
"@types/chai": "^4.1.6",
"@types/md5": "^2.2.0",
Expand All @@ -77,8 +87,11 @@
"@types/mock-require": "~2.0.0",
"@types/mock-fs": "~4.10.0",
"chai": "^4.2.0",
"concurrently": "^6.2.1",
"cross-env": "7.0.3",
"dotenv": "^8.2.0",
"eslint": "^7.15.0",
"express": "^4.16.3",
"karma": "^6.2.0",
"karma-chrome-launcher": "^3.0.0",
"karma-coverage": "^2.0.0",
Expand Down Expand Up @@ -106,6 +119,7 @@
"rollup-plugin-terser": "^5.1.1",
"rollup-plugin-visualizer": "^4.0.4",
"tslib": "^2.2.0",
"ts-node": "^9.0.0",
"typescript": "~4.2.0",
"xhr-mock": "^2.4.1"
}
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions sdk/test-utils/recorder-new/src/core-v1-recorder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { HttpClient, HttpOperationResponse } from "@azure/core-http";
import { DefaultHttpClient, WebResourceLike } from "@azure/core-http";
import { isPlaybackMode, isRecordMode } from "@azure-tools/test-recorder";
import { TestProxyHttpClient } from "./core-v2-recorder";
import { Test } from "mocha";

/**
* This client manages the recorder life cycle and interacts with the proxy-tool to do the recording,
Expand All @@ -15,8 +16,8 @@ import { TestProxyHttpClient } from "./core-v2-recorder";
*/
export class TestProxyHttpClientCoreV1 extends TestProxyHttpClient {
public httpClientCoreV1: HttpClient;
constructor(sessionFile: string) {
super(sessionFile);
constructor(testContext?: Test) {
super(testContext);
this.httpClientCoreV1 = new DefaultHttpClient();
}

Expand Down
44 changes: 35 additions & 9 deletions sdk/test-utils/recorder-new/src/core-v2-recorder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ import {
SendRequest
} from "@azure/core-rest-pipeline";
import { env, isPlaybackMode, isRecordMode } from "@azure-tools/test-recorder";
import { RecorderError, RecordingStateManager } from "./utils";
import { RecorderError, RecordingStateManager } from "./utils/utils";
import { Test } from "mocha";
import { sessionFilePath } from "./utils/sessionFilePath";

const paths = {
playback: "/playback",
Expand All @@ -32,15 +34,22 @@ export class TestProxyHttpClient {
private url = "http://localhost:5000";
public recordingId?: string;
public mode: string;
public httpClient: HttpClient;
private stateManager = new RecordingStateManager();
private playback: boolean;
public httpClient: HttpClient | undefined = undefined;
private sessionFile: string | undefined = undefined;

constructor(private sessionFile: string) {
this.sessionFile = sessionFile;
constructor(private testContext?: Test | undefined) {
this.mode = env.TEST_MODE;
this.playback = isPlaybackMode();
this.httpClient = createDefaultHttpClient();
if (isRecordMode() || isPlaybackMode()) {
if (this.testContext) {
this.sessionFile = sessionFilePath(this.testContext);
this.httpClient = createDefaultHttpClient();
} else {
throw new Error(
"Unable to determine the recording file path, testContext provided is not defined."
);
}
}
}

/**
Expand Down Expand Up @@ -99,10 +108,15 @@ export class TestProxyHttpClient {
if (isPlaybackMode() || isRecordMode()) {
this.stateManager.state = "started";
if (this.recordingId === undefined) {
const startUri = `${this.url}${this.playback ? paths.playback : paths.record}${
const startUri = `${this.url}${isPlaybackMode() ? paths.playback : paths.record}${
paths.start
}`;
const req = this._createRecordingRequest(startUri);
if (!this.httpClient) {
throw new RecorderError(
`Something went wrong, TestProxyHttpClient.httpClient should not have been undefined in ${this.mode} mode.`
);
}
const rsp = await this.httpClient.sendRequest({
...req,
allowInsecureConnection: true
Expand All @@ -126,10 +140,17 @@ export class TestProxyHttpClient {
if (isPlaybackMode() || isRecordMode()) {
this.stateManager.state = "stopped";
if (this.recordingId !== undefined) {
const stopUri = `${this.url}${this.playback ? paths.playback : paths.record}${paths.stop}`;
const stopUri = `${this.url}${isPlaybackMode() ? paths.playback : paths.record}${
paths.stop
}`;
const req = this._createRecordingRequest(stopUri);
req.headers.set("x-recording-save", "true");

if (!this.httpClient) {
throw new RecorderError(
`Something went wrong, TestProxyHttpClient.httpClient should not have been undefined in ${this.mode} mode.`
);
}
const rsp = await this.httpClient.sendRequest({
...req,
allowInsecureConnection: true
Expand All @@ -152,6 +173,11 @@ export class TestProxyHttpClient {
*/
private _createRecordingRequest(url: string) {
const req = createPipelineRequest({ url: url, method: "POST" });
if (!this.sessionFile) {
throw new RecorderError(
`Something went wrong, TestProxyHttpClient.sessionFile should not have been undefined in ${this.mode} mode.`
);
}
req.headers.set("x-recording-file", this.sessionFile);
if (this.recordingId !== undefined) {
req.headers.set("x-recording-id", this.recordingId);
Expand Down
1 change: 1 addition & 0 deletions sdk/test-utils/recorder-new/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@

export { recorderHttpPolicy, TestProxyHttpClient } from "./core-v2-recorder";
export { TestProxyHttpClientCoreV1 } from "./core-v1-recorder";
export { relativeRecordingsPath } from "./utils/relativePathCalculator";
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

export function relativeRecordingsPath() {
throw new Error("Attempted to use the function meant for node in a browser.");
}
67 changes: 67 additions & 0 deletions sdk/test-utils/recorder-new/src/utils/relativePathCalculator.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

import path from "path";
import fs from "fs";
import { RecorderError } from "./utils";

/**
* ONLY WORKS IN THE NODE.JS ENVIRONMENT
*
* Returns the potential `recordings` folder(relative path) for the project using `process.cwd()`.
*
* Note for browser tests:
* 1. Supposed to be called from karma.conf.js in the package for which the testing is being done.
* 2. Set this `RECORDINGS_RELATIVE_PATH` as an env variable
* ```js
* const { relativeRecordingsPathForBrowser } = require("@azure-tools/test-recorder-new");
* process.env.RECORDINGS_RELATIVE_PATH = relativeRecordingsPathForBrowser();
* ```
* 3. Add "RECORDINGS_RELATIVE_PATH" in the `envPreprocessor` array to let this be loaded in the browser environment.
* ```
* envPreprocessor: ["RECORDINGS_RELATIVE_PATH"],
* ```
*
* `RECORDINGS_RELATIVE_PATH` in the browser environment is used in the recorder to tell the proxy-tool about the location to generate the browser recordings at.
*
* @export
* @returns {string} location of the relative `recordings` folder path - `sdk/storage/storage-blob/recordings/` example
*/
export function relativeRecordingsPath() {
let currentPath = process.cwd(); // Gives the current working directory
console.log(currentPath);

let rootPath = undefined;
let expectedProjectPath = undefined;

if (fs.existsSync(path.join(currentPath, "package.json"))) {
// <root>/sdk/service/project/package.json
if (fs.existsSync(path.join(currentPath, "package.json"))) {
expectedProjectPath = currentPath; // <root>/sdk/service/project/
const expectedRootPath = path.join(currentPath, "..", "..", ".."); // <root>/
if (
fs.existsSync(path.join(expectedRootPath, "sdk/")) && // <root>/sdk
fs.existsSync(path.join(expectedRootPath, "rush.json")) // <root>/rush.json
) {
// reached root path
rootPath = expectedRootPath;
}
}
} else {
throw new RecorderError(`'package.json' is not found at ${currentPath}`);
}

if (!(rootPath === undefined || expectedProjectPath === undefined)) {
// <root>/
// <root>/sdk/service/project/
return path
.join(path.relative(rootPath, expectedProjectPath), "recordings")
.split(path.sep)
.join(path.posix.sep); // Converting "\" to "/" (needed for windows) so that the path.sep("\") is not treated as an escape character in the browsers
// => sdk/service/project/recordings
} else {
throw new RecorderError(
"rootPath or expectedProjectPath could not be calculated properly from process.cwd()"
);
}
}
Loading

0 comments on commit e099b12

Please sign in to comment.