Skip to content

Commit 0cacd21

Browse files
kashish2508kashish2508
authored
fix(playwrighttesting): multiple runId issue for scalable runs (#31364)
### Packages impacted by this PR `@azure/microsoft-playwright-testing` ### Issues associated with this PR ### Describe the problem that is addressed by this PR ### What are the possible designs available to address the problem? If there are more than one possible design, why was the one in this PR chosen? ### Are there test cases added in this PR? _(If not, why?)_ ### Provide a list of related PRs _(if any)_ ### Command used to generate this PR:**_(Applicable only to SDK release request PRs)_ ### Checklists - [ ] Added impacted package name to the issue description - [ ] Does this PR needs any fixes in the SDK Generator?** _(If so, create an Issue in the [Autorest/typescript](https://github.com/Azure/autorest.typescript) repository and link it here)_ - [ ] Added a changelog (if necessary)
1 parent 113b188 commit 0cacd21

File tree

8 files changed

+68
-27
lines changed

8 files changed

+68
-27
lines changed

‎sdk/playwrighttesting/microsoft-playwright-testing/review/microsoft-playwright-testing.api.md‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,6 @@ export const ServiceAuth: {
8080
// @public
8181
export const ServiceEnvironmentVariable: {
8282
PLAYWRIGHT_SERVICE_OS: string;
83-
PLAYWRIGHT_SERVICE_RUN_ID: string;
8483
PLAYWRIGHT_SERVICE_EXPOSE_NETWORK_ENVIRONMENT_VARIABLE: string;
8584
PLAYWRIGHT_SERVICE_ACCESS_TOKEN: string;
8685
PLAYWRIGHT_SERVICE_URL: string;

‎sdk/playwrighttesting/microsoft-playwright-testing/src/common/constants.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ export const ServiceAuth = {
3232
*/
3333
export const ServiceEnvironmentVariable = {
3434
PLAYWRIGHT_SERVICE_OS: "PLAYWRIGHT_SERVICE_OS",
35-
PLAYWRIGHT_SERVICE_RUN_ID: "PLAYWRIGHT_SERVICE_RUN_ID",
3635
PLAYWRIGHT_SERVICE_EXPOSE_NETWORK_ENVIRONMENT_VARIABLE: "PLAYWRIGHT_SERVICE_EXPOSE_NETWORK",
3736
PLAYWRIGHT_SERVICE_ACCESS_TOKEN: "PLAYWRIGHT_SERVICE_ACCESS_TOKEN",
3837
PLAYWRIGHT_SERVICE_URL: "PLAYWRIGHT_SERVICE_URL",
@@ -239,6 +238,7 @@ export const TestResultErrorConstants = [
239238
export const InternalEnvironmentVariables = {
240239
MPT_PLAYWRIGHT_VERSION: "_MPT_PLAYWRIGHT_VERSION",
241240
MPT_SETUP_FATAL_ERROR: "_MPT_SETUP_FATAL_ERROR",
241+
MPT_SERVICE_RUN_ID: "_MPT_SERVICE_RUN_ID",
242242
MPT_CLOUD_HOSTED_BROWSER_USED: "_MPT_CLOUD_HOSTED_BROWSER_USED",
243243
};
244244

‎sdk/playwrighttesting/microsoft-playwright-testing/src/common/environmentVariables.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the MIT License.
33

44
import { randomUUID } from "crypto";
5+
import { InternalEnvironmentVariables } from "./constants";
56

67
export class EnvironmentVariables {
78
get accessToken(): string {
@@ -15,7 +16,7 @@ export class EnvironmentVariables {
1516
shardId: string | undefined;
1617
region: string | undefined;
1718
constructor() {
18-
this.runId = process.env["PLAYWRIGHT_SERVICE_RUN_ID"]!;
19+
this.runId = process.env[InternalEnvironmentVariables.MPT_SERVICE_RUN_ID]!;
1920
this.correlationId = randomUUID();
2021
}
2122
}

‎sdk/playwrighttesting/microsoft-playwright-testing/src/common/playwrightServiceConfig.ts‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,13 @@
11
// Copyright (c) Microsoft Corporation.
22
// Licensed under the MIT License.
33

4-
import { DefaultConnectOptionsConstants, ServiceEnvironmentVariable } from "./constants";
4+
import {
5+
DefaultConnectOptionsConstants,
6+
InternalEnvironmentVariables,
7+
ServiceEnvironmentVariable,
8+
} from "./constants";
59
import { PlaywrightServiceAdditionalOptions, OsType } from "./types";
6-
import { getDefaultRunId } from "../utils/utils";
10+
import { getAndSetRunId } from "../utils/utils";
711

812
class PlaywrightServiceConfig {
913
public serviceOs: OsType;
@@ -15,8 +19,7 @@ class PlaywrightServiceConfig {
1519
constructor() {
1620
this.serviceOs = (process.env[ServiceEnvironmentVariable.PLAYWRIGHT_SERVICE_OS] ||
1721
DefaultConnectOptionsConstants.DEFAULT_SERVICE_OS) as OsType;
18-
this.runId =
19-
process.env[ServiceEnvironmentVariable.PLAYWRIGHT_SERVICE_RUN_ID] || getDefaultRunId();
22+
this.runId = process.env[InternalEnvironmentVariables.MPT_SERVICE_RUN_ID] || "";
2023
this.timeout = DefaultConnectOptionsConstants.DEFAULT_TIMEOUT;
2124
this.slowMo = DefaultConnectOptionsConstants.DEFAULT_SLOW_MO;
2225
this.exposeNetwork = DefaultConnectOptionsConstants.DEFAULT_EXPOSE_NETWORK;
@@ -26,9 +29,13 @@ class PlaywrightServiceConfig {
2629
if (options?.exposeNetwork) {
2730
this.exposeNetwork = options.exposeNetwork;
2831
}
29-
if (options?.runId) {
30-
this.runId = options.runId;
31-
process.env[ServiceEnvironmentVariable.PLAYWRIGHT_SERVICE_RUN_ID] = this.runId;
32+
if (!process.env[InternalEnvironmentVariables.MPT_SERVICE_RUN_ID]) {
33+
if (options?.runId) {
34+
this.runId = options.runId;
35+
process.env[InternalEnvironmentVariables.MPT_SERVICE_RUN_ID] = this.runId;
36+
} else {
37+
this.runId = getAndSetRunId();
38+
}
3239
}
3340
if (options?.os) {
3441
this.serviceOs = options.os;

‎sdk/playwrighttesting/microsoft-playwright-testing/src/utils/utils.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,9 @@ export const getServiceBaseURL = (): string | undefined => {
6262
return process.env[ServiceEnvironmentVariable.PLAYWRIGHT_SERVICE_URL];
6363
};
6464

65-
export const getDefaultRunId = (): string => {
65+
export const getAndSetRunId = (): string => {
6666
const runId = ReporterUtils.getRunId(CIInfoProvider.getCIInfo());
67-
process.env[ServiceEnvironmentVariable.PLAYWRIGHT_SERVICE_RUN_ID] = runId;
67+
process.env[InternalEnvironmentVariables.MPT_SERVICE_RUN_ID] = runId;
6868
return runId;
6969
};
7070

‎sdk/playwrighttesting/microsoft-playwright-testing/test/common/playwrightServiceConfig.spec.ts‎

Lines changed: 43 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,13 @@
33

44
import {
55
DefaultConnectOptionsConstants,
6+
InternalEnvironmentVariables,
67
ServiceEnvironmentVariable,
78
} from "../../src/common/constants";
89
import { PlaywrightServiceConfig } from "../../src/common/playwrightServiceConfig";
910
import { expect } from "@azure-tools/test-utils";
1011
import sinon from "sinon";
12+
import { getAndSetRunId } from "../../src/utils/utils";
1113

1214
describe("PlaywrightServiceConfig", () => {
1315
let sandbox: sinon.SinonSandbox;
@@ -32,32 +34,30 @@ describe("PlaywrightServiceConfig", () => {
3234
expect(playwrightServiceConfig.serviceOs).to.equal(
3335
DefaultConnectOptionsConstants.DEFAULT_SERVICE_OS,
3436
);
35-
expect(playwrightServiceConfig.runId).to.exist;
37+
expect(playwrightServiceConfig.runId).to.equal("");
3638
expect(playwrightServiceConfig.timeout).to.equal(
3739
DefaultConnectOptionsConstants.DEFAULT_TIMEOUT,
3840
);
3941
expect(playwrightServiceConfig.slowMo).to.equal(DefaultConnectOptionsConstants.DEFAULT_SLOW_MO);
4042
expect(playwrightServiceConfig.exposeNetwork).to.equal(
4143
DefaultConnectOptionsConstants.DEFAULT_EXPOSE_NETWORK,
4244
);
43-
expect(process.env[ServiceEnvironmentVariable.PLAYWRIGHT_SERVICE_RUN_ID]).to.equal(
44-
playwrightServiceConfig.runId,
45-
);
45+
expect(process.env[InternalEnvironmentVariables.MPT_SERVICE_RUN_ID]).to.undefined;
4646

47-
delete process.env[ServiceEnvironmentVariable.PLAYWRIGHT_SERVICE_RUN_ID];
47+
delete process.env[InternalEnvironmentVariables.MPT_SERVICE_RUN_ID];
4848
delete process.env[ServiceEnvironmentVariable.PLAYWRIGHT_SERVICE_OS];
4949
});
5050

5151
it("should set service config object with values from env variables", () => {
5252
process.env[ServiceEnvironmentVariable.PLAYWRIGHT_SERVICE_OS] = "windows";
53-
process.env[ServiceEnvironmentVariable.PLAYWRIGHT_SERVICE_RUN_ID] = "runId";
53+
process.env[InternalEnvironmentVariables.MPT_SERVICE_RUN_ID] = "runId";
5454

5555
const playwrightServiceConfig = new PlaywrightServiceConfig();
5656

5757
expect(playwrightServiceConfig.serviceOs).to.equal("windows");
5858
expect(playwrightServiceConfig.runId).to.equal("runId");
5959

60-
delete process.env[ServiceEnvironmentVariable.PLAYWRIGHT_SERVICE_RUN_ID];
60+
delete process.env[InternalEnvironmentVariables.MPT_SERVICE_RUN_ID];
6161
delete process.env[ServiceEnvironmentVariable.PLAYWRIGHT_SERVICE_OS];
6262
});
6363

@@ -76,10 +76,10 @@ describe("PlaywrightServiceConfig", () => {
7676
expect(playwrightServiceConfig.slowMo).to.equal(100);
7777
expect(playwrightServiceConfig.timeout).to.equal(200);
7878
expect(playwrightServiceConfig.exposeNetwork).to.equal("localhost");
79-
expect(process.env[ServiceEnvironmentVariable.PLAYWRIGHT_SERVICE_RUN_ID]).to.equal("runId");
79+
expect(process.env[InternalEnvironmentVariables.MPT_SERVICE_RUN_ID]).to.equal("runId");
8080
expect(process.env[ServiceEnvironmentVariable.PLAYWRIGHT_SERVICE_OS]).to.equal("windows");
8181

82-
delete process.env[ServiceEnvironmentVariable.PLAYWRIGHT_SERVICE_RUN_ID];
82+
delete process.env[InternalEnvironmentVariables.MPT_SERVICE_RUN_ID];
8383
delete process.env[ServiceEnvironmentVariable.PLAYWRIGHT_SERVICE_OS];
8484
});
8585

@@ -98,11 +98,43 @@ describe("PlaywrightServiceConfig", () => {
9898
expect(playwrightServiceConfig.exposeNetwork).to.equal(
9999
DefaultConnectOptionsConstants.DEFAULT_EXPOSE_NETWORK,
100100
);
101-
expect(process.env[ServiceEnvironmentVariable.PLAYWRIGHT_SERVICE_RUN_ID]).to.equal(
101+
expect(process.env[InternalEnvironmentVariables.MPT_SERVICE_RUN_ID]).to.equal(
102102
playwrightServiceConfig.runId,
103103
);
104104

105-
delete process.env[ServiceEnvironmentVariable.PLAYWRIGHT_SERVICE_RUN_ID];
105+
delete process.env[InternalEnvironmentVariables.MPT_SERVICE_RUN_ID];
106106
delete process.env[ServiceEnvironmentVariable.PLAYWRIGHT_SERVICE_OS];
107107
});
108+
it("should set runId from options if provided and environment variable is not set", () => {
109+
const playwrightServiceConfig = new PlaywrightServiceConfig();
110+
playwrightServiceConfig.setOptions({
111+
runId: "custom-run-id",
112+
});
113+
expect(playwrightServiceConfig.runId).to.equal("custom-run-id");
114+
expect(process.env[InternalEnvironmentVariables.MPT_SERVICE_RUN_ID]).to.equal("custom-run-id");
115+
delete process.env[InternalEnvironmentVariables.MPT_SERVICE_RUN_ID];
116+
});
117+
it("should generate runId if not provided in options and environment variable is not set", () => {
118+
const runId = getAndSetRunId();
119+
const playwrightServiceConfig = new PlaywrightServiceConfig();
120+
playwrightServiceConfig.setOptions({
121+
runId: runId,
122+
});
123+
expect(playwrightServiceConfig.runId).to.be.a("string");
124+
expect(playwrightServiceConfig.runId).to.equal(runId);
125+
expect(process.env[InternalEnvironmentVariables.MPT_SERVICE_RUN_ID]).to.equal(runId);
126+
delete process.env[InternalEnvironmentVariables.MPT_SERVICE_RUN_ID];
127+
});
128+
it("should use runId from environment variable if already set", () => {
129+
process.env[InternalEnvironmentVariables.MPT_SERVICE_RUN_ID] = "existing-run-id";
130+
const playwrightServiceConfig = new PlaywrightServiceConfig();
131+
playwrightServiceConfig.setOptions({
132+
runId: "option-run-id",
133+
});
134+
expect(playwrightServiceConfig.runId).to.equal("existing-run-id");
135+
expect(process.env[InternalEnvironmentVariables.MPT_SERVICE_RUN_ID]).to.equal(
136+
"existing-run-id",
137+
);
138+
delete process.env[InternalEnvironmentVariables.MPT_SERVICE_RUN_ID];
139+
});
108140
});

‎sdk/playwrighttesting/microsoft-playwright-testing/test/core/playwrightService.spec.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ describe("getServiceConfig", () => {
6666
});
6767

6868
it("should set service config options as passed", () => {
69+
delete process.env[InternalEnvironmentVariables.MPT_SERVICE_RUN_ID];
6970
const { getServiceConfig } = require("../../src/core/playwrightService");
7071
getServiceConfig(samplePlaywrightConfigInput, {
7172
os: ServiceOS.WINDOWS,
@@ -185,6 +186,7 @@ describe("getConnectOptions", () => {
185186
});
186187

187188
it("should set service connect options with passed values", async () => {
189+
delete process.env[InternalEnvironmentVariables.MPT_SERVICE_RUN_ID];
188190
const { getConnectOptions } = require("../../src/core/playwrightService");
189191
await getConnectOptions({
190192
runId: "1234",

‎sdk/playwrighttesting/microsoft-playwright-testing/test/utils/utils.spec.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import * as utils from "../../src/utils/utils";
1111
import {
1212
getAccessToken,
1313
getServiceBaseURL,
14-
getDefaultRunId,
14+
getAndSetRunId,
1515
getServiceWSEndpoint,
1616
validateServiceUrl,
1717
validateMptPAT,
@@ -63,11 +63,11 @@ describe("Service Utils", () => {
6363
});
6464

6565
it("should return and set run id set in env variable", () => {
66-
const runId = getDefaultRunId();
66+
const runId = getAndSetRunId();
6767
expect(runId).to.be.a("string");
68-
expect(process.env[ServiceEnvironmentVariable.PLAYWRIGHT_SERVICE_RUN_ID]).to.equal(runId);
68+
expect(process.env[InternalEnvironmentVariables.MPT_SERVICE_RUN_ID]).to.equal(runId);
6969

70-
delete process.env[ServiceEnvironmentVariable.PLAYWRIGHT_SERVICE_RUN_ID];
70+
delete process.env[InternalEnvironmentVariables.MPT_SERVICE_RUN_ID];
7171
});
7272

7373
it("should return service base url with query params", () => {

0 commit comments

Comments
 (0)