Skip to content

Commit 2df6e0b

Browse files
lfkellogglfkellogg
committed
Address feedback from @joehan and go/atad-cli-api-review
1 parent 6fb713f commit 2df6e0b

5 files changed

Lines changed: 140 additions & 133 deletions

File tree

‎src/appdistribution/client.ts‎

Lines changed: 10 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -6,113 +6,16 @@ import { Distribution } from "./distribution";
66
import { FirebaseError } from "../error";
77
import { Client } from "../apiv2";
88
import { appDistributionOrigin } from "../api";
9-
10-
/**
11-
* Helper interface for an app that is provisioned with App Distribution
12-
*/
13-
export interface AabInfo {
14-
name: string;
15-
integrationState: IntegrationState;
16-
testCertificate: TestCertificate | null;
17-
}
18-
19-
export interface TestCertificate {
20-
hashSha1: string;
21-
hashSha256: string;
22-
hashMd5: string;
23-
}
24-
25-
/** Enum representing the App Bundles state for the App */
26-
export enum IntegrationState {
27-
AAB_INTEGRATION_STATE_UNSPECIFIED = "AAB_INTEGRATION_STATE_UNSPECIFIED",
28-
INTEGRATED = "INTEGRATED",
29-
PLAY_ACCOUNT_NOT_LINKED = "PLAY_ACCOUNT_NOT_LINKED",
30-
NO_APP_WITH_GIVEN_BUNDLE_ID_IN_PLAY_ACCOUNT = "NO_APP_WITH_GIVEN_BUNDLE_ID_IN_PLAY_ACCOUNT",
31-
APP_NOT_PUBLISHED = "APP_NOT_PUBLISHED",
32-
AAB_STATE_UNAVAILABLE = "AAB_STATE_UNAVAILABLE",
33-
PLAY_IAS_TERMS_NOT_ACCEPTED = "PLAY_IAS_TERMS_NOT_ACCEPTED",
34-
}
35-
36-
export enum UploadReleaseResult {
37-
UPLOAD_RELEASE_RESULT_UNSPECIFIED = "UPLOAD_RELEASE_RESULT_UNSPECIFIED",
38-
RELEASE_CREATED = "RELEASE_CREATED",
39-
RELEASE_UPDATED = "RELEASE_UPDATED",
40-
RELEASE_UNMODIFIED = "RELEASE_UNMODIFIED",
41-
}
42-
43-
export interface Release {
44-
name: string;
45-
releaseNotes: ReleaseNotes;
46-
displayVersion: string;
47-
buildVersion: string;
48-
createTime: Date;
49-
firebaseConsoleUri: string;
50-
testingUri: string;
51-
binaryDownloadUri: string;
52-
}
53-
54-
export interface ReleaseNotes {
55-
text: string;
56-
}
57-
58-
export interface UploadReleaseResponse {
59-
result: UploadReleaseResult;
60-
release: Release;
61-
}
62-
63-
export interface BatchRemoveTestersResponse {
64-
emails: string[];
65-
}
66-
67-
export interface Group {
68-
name: string;
69-
displayName: string;
70-
testerCount?: number;
71-
releaseCount?: number;
72-
inviteLinkCount?: number;
73-
}
74-
75-
export interface CreateReleaseTestRequest {
76-
releaseTest: ReleaseTest;
77-
}
78-
79-
export enum TestState {
80-
IN_PROGRESS = "IN_PROGRESS",
81-
PASSED = "PASSED",
82-
FAILED = "FAILED",
83-
INCONCLUSIVE = "INCONCLUSIVE",
84-
}
85-
86-
export interface TestDevice {
87-
model: string;
88-
version: string;
89-
locale: string;
90-
orientation: string;
91-
}
92-
93-
export interface DeviceExecution {
94-
device: TestDevice;
95-
state?: TestState;
96-
failedReason?: string;
97-
inconclusiveReason?: string;
98-
}
99-
100-
export interface FieldHints {
101-
usernameResourceName?: string;
102-
passwordResourceName?: string;
103-
}
104-
105-
export interface LoginCredential {
106-
username?: string;
107-
password?: string;
108-
fieldHints?: FieldHints;
109-
}
110-
111-
export interface ReleaseTest {
112-
name?: string;
113-
deviceExecutions: DeviceExecution[];
114-
loginCredential?: LoginCredential;
115-
}
9+
import {
10+
AabInfo,
11+
BatchRemoveTestersResponse,
12+
DeviceExecution,
13+
Group,
14+
LoginCredential,
15+
ReleaseTest,
16+
TestDevice,
17+
UploadReleaseResponse,
18+
} from "./types";
11619

11720
/**
11821
* Makes RPCs to the App Distribution server backend.
@@ -321,7 +224,6 @@ export class AppDistributionClient {
321224
loginCredential,
322225
},
323226
});
324-
utils.logSuccess(`Release test created successfully`);
325227
return response.body;
326228
} catch (err: any) {
327229
throw new FirebaseError(`Failed to create release test ${err}`);

‎src/appdistribution/options-parser-util.ts‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import * as fs from "fs-extra";
22
import { FirebaseError } from "../error";
33
import { needProjectNumber } from "../projectUtils";
4-
import { FieldHints, LoginCredential, TestDevice } from "./client";
4+
import { FieldHints, LoginCredential, TestDevice } from "./types";
55

66
/**
77
* Takes in comma separated string or a path to a comma/new line separated file
@@ -81,13 +81,11 @@ export function getTestDevices(value: string, file: string): TestDevice[] {
8181
return [];
8282
}
8383

84-
// The value is split into a string[]
85-
const deviceStrings = value
84+
return value
8685
.split(/[;\n]/)
8786
.map((entry) => entry.trim())
88-
.filter((entry) => !!entry);
89-
90-
return deviceStrings.map((str) => parseTestDevice(str));
87+
.filter((entry) => !!entry)
88+
.map((str) => parseTestDevice(str));
9189
}
9290

9391
function parseTestDevice(testDeviceString: string): TestDevice {
@@ -134,9 +132,15 @@ function parseTestDevice(testDeviceString: string): TestDevice {
134132
export function getLoginCredential(
135133
username?: string,
136134
password?: string,
135+
passwordFile?: string,
137136
usernameResourceName?: string,
138137
passwordResourceName?: string,
139138
): LoginCredential | undefined {
139+
if (!password && passwordFile) {
140+
ensureFileExists(passwordFile);
141+
password = fs.readFileSync(passwordFile, "utf8");
142+
}
143+
140144
if (isPresenceMismatched(usernameResourceName, passwordResourceName)) {
141145
throw new FirebaseError(
142146
"Username and password resource names for automated tests need to be specified together.",

‎src/appdistribution/types.ts‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
/**
2+
* Helper interface for an app that is provisioned with App Distribution
3+
*/
4+
export interface AabInfo {
5+
name: string;
6+
integrationState: IntegrationState;
7+
testCertificate: TestCertificate | null;
8+
}
9+
10+
export interface TestCertificate {
11+
hashSha1: string;
12+
hashSha256: string;
13+
hashMd5: string;
14+
}
15+
16+
/** Enum representing the App Bundles state for the App */
17+
export enum IntegrationState {
18+
AAB_INTEGRATION_STATE_UNSPECIFIED = "AAB_INTEGRATION_STATE_UNSPECIFIED",
19+
INTEGRATED = "INTEGRATED",
20+
PLAY_ACCOUNT_NOT_LINKED = "PLAY_ACCOUNT_NOT_LINKED",
21+
NO_APP_WITH_GIVEN_BUNDLE_ID_IN_PLAY_ACCOUNT = "NO_APP_WITH_GIVEN_BUNDLE_ID_IN_PLAY_ACCOUNT",
22+
APP_NOT_PUBLISHED = "APP_NOT_PUBLISHED",
23+
AAB_STATE_UNAVAILABLE = "AAB_STATE_UNAVAILABLE",
24+
PLAY_IAS_TERMS_NOT_ACCEPTED = "PLAY_IAS_TERMS_NOT_ACCEPTED",
25+
}
26+
27+
export enum UploadReleaseResult {
28+
UPLOAD_RELEASE_RESULT_UNSPECIFIED = "UPLOAD_RELEASE_RESULT_UNSPECIFIED",
29+
RELEASE_CREATED = "RELEASE_CREATED",
30+
RELEASE_UPDATED = "RELEASE_UPDATED",
31+
RELEASE_UNMODIFIED = "RELEASE_UNMODIFIED",
32+
}
33+
34+
export interface Release {
35+
name: string;
36+
releaseNotes: ReleaseNotes;
37+
displayVersion: string;
38+
buildVersion: string;
39+
createTime: Date;
40+
firebaseConsoleUri: string;
41+
testingUri: string;
42+
binaryDownloadUri: string;
43+
}
44+
45+
export interface ReleaseNotes {
46+
text: string;
47+
}
48+
49+
export interface UploadReleaseResponse {
50+
result: UploadReleaseResult;
51+
release: Release;
52+
}
53+
54+
export interface BatchRemoveTestersResponse {
55+
emails: string[];
56+
}
57+
58+
export interface Group {
59+
name: string;
60+
displayName: string;
61+
testerCount?: number;
62+
releaseCount?: number;
63+
inviteLinkCount?: number;
64+
}
65+
66+
export interface CreateReleaseTestRequest {
67+
releaseTest: ReleaseTest;
68+
}
69+
70+
export interface TestDevice {
71+
model: string;
72+
version: string;
73+
locale: string;
74+
orientation: string;
75+
}
76+
77+
export type TestState = "IN_PROGRESS" | "PASSED" | "FAILED" | "INCONCLUSIVE";
78+
79+
export interface DeviceExecution {
80+
device: TestDevice;
81+
state?: TestState;
82+
failedReason?: string;
83+
inconclusiveReason?: string;
84+
}
85+
86+
export interface FieldHints {
87+
usernameResourceName?: string;
88+
passwordResourceName?: string;
89+
}
90+
91+
export interface LoginCredential {
92+
username?: string;
93+
password?: string;
94+
fieldHints?: FieldHints;
95+
}
96+
97+
export interface ReleaseTest {
98+
name?: string;
99+
deviceExecutions: DeviceExecution[];
100+
loginCredential?: LoginCredential;
101+
}

‎src/commands/appdistribution-distribute.ts‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,13 @@ import * as fs from "fs-extra";
33
import { Command } from "../command";
44
import * as utils from "../utils";
55
import { requireAuth } from "../requireAuth";
6+
import { AppDistributionClient } from "../appdistribution/client";
67
import {
78
AabInfo,
89
IntegrationState,
9-
AppDistributionClient,
1010
UploadReleaseResult,
11-
TestState,
1211
TestDevice,
13-
} from "../appdistribution/client";
12+
} from "../appdistribution/types";
1413
import { FirebaseError } from "../error";
1514
import { Distribution, DistributionFileType } from "../appdistribution/distribution";
1615
import {
@@ -59,7 +58,11 @@ export const command = new Command("appdistribution:distribute <release-binary-f
5958
"path to file containing a list of semicolon- or newline-separated devices to run automated tests on, in the format 'model=<model-id>,version=<os-version-id>,locale=<locale>,orientation=<orientation>'. Run 'gcloud firebase test android|ios models list' to see available devices. Note: This feature is in beta.",
6059
)
6160
.option("--test-username <string>", "username for automatic login")
62-
.option("--test-password <string>", "password for automatic login")
61+
.option(
62+
"--test-password <string>",
63+
"password for automatic login. If using a real password, use --test-password-file instead to avoid putting sensitive info in history and logs.",
64+
)
65+
.option("--test-password-file <string>", "path to file containing password for automatic login")
6366
.option(
6467
"--test-username-resource <string>",
6568
"resource name for the username field for automatic login",
@@ -83,6 +86,7 @@ export const command = new Command("appdistribution:distribute <release-binary-f
8386
const loginCredential = getLoginCredential(
8487
options.testUsername,
8588
options.testPassword,
89+
options.testPasswordFile,
8690
options.testUsernameResource,
8791
options.testPasswordResource,
8892
);
@@ -213,6 +217,7 @@ export const command = new Command("appdistribution:distribute <release-binary-f
213217
testDevices,
214218
loginCredential,
215219
);
220+
utils.logSuccess(`Release test created successfully`);
216221
if (!options.testAsync) {
217222
await awaitTestResults(releaseTest.name!, requests);
218223
}
@@ -227,21 +232,21 @@ async function awaitTestResults(
227232
utils.logBullet("the automated tests results are pending");
228233
await delay(TEST_POLLING_INTERVAL_MILLIS);
229234
const releaseTest = await requests.getReleaseTest(releaseTestName);
230-
if (releaseTest.deviceExecutions.every((e) => e.state === TestState.PASSED)) {
235+
if (releaseTest.deviceExecutions.every((e) => e.state === "PASSED")) {
231236
utils.logSuccess("automated test(s) passed!");
232237
return;
233238
}
234239
for (const execution of releaseTest.deviceExecutions) {
235240
switch (execution.state) {
236-
case TestState.PASSED:
237-
case TestState.IN_PROGRESS:
241+
case "PASSED":
242+
case "IN_PROGRESS":
238243
continue;
239-
case TestState.FAILED:
244+
case "FAILED":
240245
throw new FirebaseError(
241246
`Automated test failed for ${deviceToString(execution.device)}: ${execution.failedReason}`,
242247
{ exit: 1 },
243248
);
244-
case TestState.INCONCLUSIVE:
249+
case "INCONCLUSIVE":
245250
throw new FirebaseError(
246251
`Automated test inconclusive for ${deviceToString(execution.device)}: ${execution.inconclusiveReason}`,
247252
{ exit: 1 },
@@ -259,7 +264,7 @@ async function awaitTestResults(
259264
});
260265
}
261266

262-
function delay(ms: number) {
267+
function delay(ms: number): Promise<number> {
263268
return new Promise((resolve) => setTimeout(resolve, ms));
264269
}
265270

‎src/test/appdistro/client.spec.ts‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,8 @@ import * as rimraf from "rimraf";
66
import * as sinon from "sinon";
77
import * as tmp from "tmp";
88

9-
import {
10-
AppDistributionClient,
11-
BatchRemoveTestersResponse,
12-
Group,
13-
TestDevice,
14-
TestState,
15-
} from "../../appdistribution/client";
9+
import { AppDistributionClient } from "../../appdistribution/client";
10+
import { BatchRemoveTestersResponse, Group, TestDevice } from "../../appdistribution/types";
1611
import { appDistributionOrigin } from "../../api";
1712
import { Distribution } from "../../appdistribution/distribution";
1813
import { FirebaseError } from "../../error";
@@ -322,7 +317,7 @@ describe("distribution", () => {
322317
const mockReleaseTest = {
323318
name: `${releaseName}/tests/fake-test-id`,
324319
devices: mockDevices,
325-
state: TestState.IN_PROGRESS,
320+
state: "IN_PROGRESS",
326321
};
327322

328323
it("should throw error if request fails", async () => {
@@ -363,7 +358,7 @@ describe("distribution", () => {
363358
const mockReleaseTest = {
364359
name: releaseTestName,
365360
devices: mockDevices,
366-
state: TestState.IN_PROGRESS,
361+
state: "IN_PROGRESS",
367362
};
368363

369364
it("should throw error if request fails", async () => {

0 commit comments

Comments
 (0)