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

[recorder] Figure out assets.json from RELATIVE_RECORDINGS_PATH #24776

Merged
merged 3 commits into from
Feb 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions sdk/test-utils/recorder/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
- Allow mapping the test-proxy tool to ports other than just 5000(for HTTP) using the environment variable `TEST_PROXY_HTTP_PORT`(and `TEST_PROXY_HTTPS_PORT` for 5001(for HTTPS)).
- If `TEST_PROXY_HTTP_PORT` is undefined, we'll try for 5000 as usual.
- For browsers, this variable has to be added as part of the environment variables listed under `envPreprocessor` array in `karma.conf.js` so that the recorder knows the port to hit.
- Added support for the asset sync tool. If an `assets.json` exists in the package directory, the recorder will fetch recordings from the external repo.

### Breaking Changes

Expand Down
3 changes: 1 addition & 2 deletions sdk/test-utils/recorder/karma.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ process.env.CHROME_BIN = require("puppeteer").executablePath();
require("dotenv").config({ path: "../.env" });

process.env.RECORDINGS_RELATIVE_PATH = relativeRecordingsPath();
process.env.RECORDING_ASSETS_PATH = relativeAssetsPath();
timovv marked this conversation as resolved.
Show resolved Hide resolved

module.exports = function (config) {
config.set({
Expand Down Expand Up @@ -50,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: ["RECORDINGS_RELATIVE_PATH", "PROXY_MANUAL_START", "RECORDING_ASSETS_PATH"],
envPreprocessor: ["RECORDINGS_RELATIVE_PATH"],

// test results reporter to use
// possible values: 'dots', 'progress'
Expand Down
2 changes: 1 addition & 1 deletion sdk/test-utils/recorder/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Licensed under the MIT license.

export { Recorder } from "./recorder";
export { relativeRecordingsPath, relativeAssetsPath } from "./utils/relativePathCalculator";
export { relativeRecordingsPath } from "./utils/relativePathCalculator";
export {
SanitizerOptions,
RecorderStartOptions,
Expand Down
36 changes: 32 additions & 4 deletions sdk/test-utils/recorder/src/recorder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {
RecordingStateManager,
} from "./utils/utils";
import { Test } from "mocha";
import { sessionFilePath } from "./utils/sessionFilePath";
import { assetsJsonPath, sessionFilePath } from "./utils/sessionFilePath";
import { SanitizerOptions } from "./utils/utils";
import { paths } from "./utils/paths";
import { addSanitizers, transformsInfo } from "./sanitizer";
Expand All @@ -43,7 +43,6 @@ import { isNode } from "@azure/core-util";
import { env } from "./utils/env";
import { decodeBase64 } from "./utils/encoding";
import { isHttpHeadersLike } from "./utils/typeGuards";
import { relativeAssetsPath } from "./utils/relativePathCalculator";

/**
* This client manages the recorder life cycle and interacts with the proxy-tool to do the recording,
Expand All @@ -70,7 +69,7 @@ export class Recorder {
if (isRecordMode() || isPlaybackMode()) {
if (this.testContext) {
this.sessionFile = sessionFilePath(this.testContext);
this.assetsJson = relativeAssetsPath();
this.assetsJson = assetsJsonPath();

logger.info(`[Recorder#constructor] Using a session file located at ${this.sessionFile}`);
this.httpClient = createDefaultHttpClient();
Expand Down Expand Up @@ -260,10 +259,39 @@ export class Recorder {
logger.verbose("[Recorder#start] Setting redirect mode");
await setRecordingOptions(Recorder.url, this.httpClient, { handleRedirects: !isNode });
logger.verbose("[Recorder#start] Sending the start request to the test proxy");
const rsp = await this.httpClient.sendRequest({
let rsp = await this.httpClient.sendRequest({
...req,
allowInsecureConnection: true,
});

// If the error is due to the assets.json not existing, try again without specifying an assets.json. This will
// occur with SDKs that have not migrated to asset sync yet.
// TODO: remove once everyone has migrated to asset sync
timovv marked this conversation as resolved.
Show resolved Hide resolved
if (rsp.status === 400 && rsp.headers.get("x-request-known-exception") === "true") {
const errorMessage = decodeBase64(rsp.headers.get("x-request-known-exception-error")!);
if (
errorMessage.includes("The provided assets") &&
errorMessage.includes("does not exist")
) {
logger.info(
"[Recorder#start] start request failed, trying again without assets.json specified"
);

const retryRequest = createRecordingRequest(
startUri,
this.sessionFile,
this.recordingId,
"POST",
undefined
);

rsp = await this.httpClient.sendRequest({
...retryRequest,
allowInsecureConnection: true,
});
}
}

if (rsp.status !== 200) {
logger.error("[Recorder#start] Could not start the recorder", rsp);
throw new RecorderError("Start request failed.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,30 +13,3 @@ export function relativeRecordingsPath(): string {
);
}
}

/**
* ONLY WORKS IN THE NODE.JS ENVIRONMENT
*
* Returns the potential assets.json 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 `RECORDING_ASSETS_PATH` as an env variable
* ```js
* const { relativeRecordingsPathForBrowser } = require("@azure-tools/test-recorder-new");
* process.env.RECORDING_ASSETS_PATH = relativeRecordingsPathForBrowser();
* ```
* 3. Add "RECORDING_ASSETS_PATH" in the `envPreprocessor` array to let this be loaded in the browser environment.
* ```
* envPreprocessor: ["RECORDING_ASSETS_PATH"],
* ```
*
* `RECORDING_ASSETS_PATH` in the browser environment is used in the recorder to tell the proxy-tool about whether or not to pass additional body argument
* `x-recording-assets-file` to playback|record/Start. Doing so enables the proxy to auto-restore files from a remote location.
*
* @export
* @returns {string} location of the relative path to discovered assets.json - `sdk/storage/storage-blob/assets.json` for example.
*/
export function relativeAssetsPath(): string | undefined {
return env.RECORDING_ASSETS_PATH;
}
27 changes: 0 additions & 27 deletions sdk/test-utils/recorder/src/utils/relativePathCalculator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,30 +76,3 @@ function relativePackagePath() {
export function relativeRecordingsPath(): string {
return toSafePath(path.join(relativePackagePath(), "recordings"));
}

/**
* Returns the potential assets.json 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 `RECORDING_ASSETS_PATH` as an env variable
* ```js
* const { relativeRecordingsPathForBrowser } = require("@azure-tools/test-recorder-new");
* process.env.RECORDING_ASSETS_PATH = relativeRecordingsPathForBrowser();
* ```
* 3. Add "RECORDING_ASSETS_PATH" in the `envPreprocessor` array to let this be loaded in the browser environment.
* ```
* envPreprocessor: ["RECORDING_ASSETS_PATH"],
* ```
*
* `RECORDING_ASSETS_PATH` in the browser environment is used in the recorder to tell the proxy-tool about whether or not to pass additional body argument
* `x-recording-assets-file` to playback|record/Start. Doing so enables the proxy to auto-restore files from a remote location.
*
* @export
* @returns {string} location of the relative path to discovered assets.json - `sdk/storage/storage-blob/assets.json` for example, or undefined if the path does not exist
*/
export function relativeAssetsPath(): string | undefined {
return fs.existsSync("assets.json")
? toSafePath(path.join(relativePackagePath(), "assets.json"))
: undefined;
}
9 changes: 9 additions & 0 deletions sdk/test-utils/recorder/src/utils/sessionFilePath.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,12 @@ export function recordingFilePath(testContext: Mocha.Test): string {
testContext.title
);
}

export function assetsJsonPath(): string {
// Hacky solution using substring works around the fact that:
// 1) the relativeRecordingsPath may not exist on disk (so relativeRecordingsPath()/../assets.json might not exist either, can't use ..)
// 2) `path` (and therefore `path.dirname`) is not available in the browser.
const recordingsPath = relativeRecordingsPath();
const sdkDir = recordingsPath.substring(0, recordingsPath.lastIndexOf("/"));
return `${sdkDir}/assets.json`;
}