Skip to content

Commit 54899a9

Browse files
mbudinskymbudinsky
committed
chore: Address code review remarks
1 parent 77e467a commit 54899a9

11 files changed

Lines changed: 294 additions & 174 deletions

File tree

‎packages/mobile-test-reporter/src/http/HttpTestReporter.ts‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,8 @@ export class HttpTestReporter implements TestMonitor {
6868
* Creates a run in the collector.
6969
*/
7070
async startRun(): Promise<string> {
71-
if (this.runId) {
72-
return this.runId;
71+
if (this.isStarted) {
72+
return this.runId!;
7373
}
7474

7575
this.logger.info(`[HttpTestReporter] connecting collector=${this.collectorUrl}`);
@@ -108,19 +108,20 @@ export class HttpTestReporter implements TestMonitor {
108108
* Flushes remaining events and completes the run in the collector.
109109
*/
110110
async completeRun(success: boolean, counters?: Partial<RunCounters>): Promise<void> {
111-
if (!this.runId) {
111+
if (!this.isStarted) {
112112
return;
113113
}
114114

115115
await this.flush();
116116

117+
const runId = this.runId!;
117118
const body: RunCompleteRequest = {
118-
runId: this.runId,
119+
runId,
119120
completedAt: new Date().toISOString(),
120121
success,
121122
counters,
122123
};
123-
const resp = await globalThis.fetch(normalizeUrl(this.collectorUrl, `/runs/${this.runId}/complete`), {
124+
const resp = await globalThis.fetch(normalizeUrl(this.collectorUrl, `/runs/${runId}/complete`), {
124125
method: 'POST',
125126
headers: { 'content-type': 'application/json' },
126127
body: JSON.stringify(body),
@@ -156,7 +157,7 @@ export class HttpTestReporter implements TestMonitor {
156157
* Sends buffered events to the collector. Safe to call multiple times.
157158
*/
158159
async flush(): Promise<void> {
159-
if (!this.runId) {
160+
if (!this.isStarted) {
160161
return;
161162
}
162163

@@ -168,15 +169,16 @@ export class HttpTestReporter implements TestMonitor {
168169
return;
169170
}
170171

172+
const runId = this.runId!;
171173
this.flushing = true;
172174
try {
173175
while (this.queue.length > 0) {
174176
const chunk = this.queue.splice(0, this.batchSize);
175177
const body: RunEventBatchRequest = {
176-
runId: this.runId,
178+
runId,
177179
events: chunk,
178180
};
179-
const resp = await globalThis.fetch(normalizeUrl(this.collectorUrl, `/runs/${this.runId}/events`), {
181+
const resp = await globalThis.fetch(normalizeUrl(this.collectorUrl, `/runs/${runId}/events`), {
180182
method: 'POST',
181183
headers: { 'content-type': 'application/json' },
182184
body: JSON.stringify(body),

‎packages/mobile-test-reporter/src/http/url.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,12 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15+
import type { PlatformOS } from '../protocol';
16+
1517
/**
1618
* Adjusts a collector URL to be reachable from the current platform.
1719
*/
18-
export function adjustCollectorUrlForPlatform(collectorUrl: string, platformOS: string): string {
20+
export function adjustCollectorUrlForPlatform(collectorUrl: string, platformOS: PlatformOS): string {
1921
const withSchemeAndPort = normalizeCollectorBaseUrl(collectorUrl);
2022

2123
const normalized = withSchemeAndPort

‎packages/mobile-test-reporter/src/protocol/types.ts‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,9 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15-
export type PlatformOS = 'android' | 'ios' | string;
15+
export type PlatformOS = 'android' | 'ios';
1616

17-
export type Runtime = 'react-native' | 'cordova' | string;
17+
export type Runtime = 'react-native' | 'cordova';
1818

1919
// Note that most of the timestamp types are ISO-8601 strings for JUnit compatibility - we can change later.
2020

@@ -69,8 +69,7 @@ export type EventType =
6969
| 'TEST_FAIL'
7070
| 'TEST_SKIPPED'
7171
| 'TEST_INFO'
72-
| 'TEST_WARN'
73-
| string;
72+
| 'TEST_WARN';
7473

7574
export interface RunFailure {
7675
name?: string;

‎packages/mobile-test-runner/src/cli.ts‎

Lines changed: 73 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -66,42 +66,7 @@ async function main(): Promise<void> {
6666
printHelp(1);
6767
}
6868

69-
let host = '127.0.0.1';
70-
let port = 8137;
71-
let outDir = 'artifacts/e2e';
72-
let expectedRuns = 1;
73-
let timeoutMs = parseDurationToMs('20m');
74-
let watch = false;
75-
76-
for (let i = 0; i < args.length; i++) {
77-
const arg = args[i];
78-
const next = args[i + 1];
79-
80-
if (arg === '--host' && next) {
81-
host = next;
82-
i++;
83-
} else if (arg === '--port' && next) {
84-
port = Number(next);
85-
i++;
86-
} else if (arg === '--out' && next) {
87-
outDir = next;
88-
i++;
89-
} else if (arg === '--expected-runs' && next) {
90-
expectedRuns = Number(next);
91-
i++;
92-
} else if (arg === '--timeout' && next) {
93-
timeoutMs = parseDurationToMs(next);
94-
i++;
95-
} else if (arg === '--watch') {
96-
watch = true;
97-
} else if (arg === '--help' || arg === '-h') {
98-
printHelp(0);
99-
} else {
100-
// eslint-disable-next-line no-console
101-
console.error(`Unknown argument: ${arg}`);
102-
printHelp(1);
103-
}
104-
}
69+
let { host, port, outDir, expectedRuns, timeoutMs, watch } = parseArgs(args);
10570

10671
if (!Number.isFinite(port) || port <= 0) {
10772
throw new Error(`Invalid --port value '${port}'.`);
@@ -137,3 +102,75 @@ main().catch((e) => {
137102
console.error(e?.stack ?? String(e));
138103
process.exit(2);
139104
});
105+
106+
type CliArgs = {
107+
host: string;
108+
port: number;
109+
outDir: string;
110+
expectedRuns: number;
111+
timeoutMs: number;
112+
watch: boolean;
113+
};
114+
115+
function parseArgs(args: string[]): CliArgs {
116+
const options: CliArgs = {
117+
host: '127.0.0.1',
118+
port: 8137,
119+
outDir: 'artifacts/e2e',
120+
expectedRuns: 1,
121+
timeoutMs: parseDurationToMs('20m'),
122+
watch: false,
123+
};
124+
125+
for (let i = 0; i < args.length; ) {
126+
const arg = args[i];
127+
128+
switch (arg) {
129+
case '--host':
130+
options.host = requireValue(arg, args[i + 1]);
131+
i += 2;
132+
break;
133+
case '--port':
134+
options.port = Number(requireValue(arg, args[i + 1]));
135+
i += 2;
136+
break;
137+
case '--out':
138+
options.outDir = requireValue(arg, args[i + 1]);
139+
i += 2;
140+
break;
141+
case '--expected-runs':
142+
options.expectedRuns = Number(requireValue(arg, args[i + 1]));
143+
i += 2;
144+
break;
145+
case '--timeout':
146+
options.timeoutMs = parseDurationToMs(requireValue(arg, args[i + 1]));
147+
i += 2;
148+
break;
149+
case '--watch':
150+
options.watch = true;
151+
i += 1;
152+
break;
153+
case '--help':
154+
case '-h':
155+
printHelp(0);
156+
return options;
157+
default:
158+
// eslint-disable-next-line no-console
159+
console.error(`Unknown argument: ${arg}`);
160+
printHelp(1);
161+
return options;
162+
}
163+
}
164+
165+
return options;
166+
}
167+
168+
function requireValue(flag: string, value: string | undefined): string {
169+
if (!value) {
170+
// eslint-disable-next-line no-console
171+
console.error(`Missing value for ${flag}.`);
172+
printHelp(1);
173+
return '';
174+
}
175+
return value;
176+
}

0 commit comments

Comments
 (0)