Skip to content

Commit 7fd7668

Browse files
jochen-testingbotjochen-testingbot
committed
improvements
1 parent fa5b814 commit 7fd7668

10 files changed

Lines changed: 273 additions & 18 deletions

File tree

‎src/cli.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -405,7 +405,9 @@ program
405405
const missing: string[] = [];
406406
if (!app) missing.push('<appFile> or --app');
407407
if (flows.length === 0)
408-
missing.push('<flows...> (one or more flow files, directories, or globs)');
408+
missing.push(
409+
'<flows...> (one or more flow files, directories, or globs)',
410+
);
409411
if (missing.length > 0) {
410412
throw new TestingBotError(
411413
`Missing required argument: ${missing.join(', ')}. Run "testingbot maestro --help" for usage.`,

‎src/config/constants.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
export const POLLING = {
22
MIN_INTERVAL_MS: 5000,
3-
MAX_INTERVAL_MS: 30000,
4-
BACKOFF_MULTIPLIER: 1.5,
3+
MAX_INTERVAL_MS: 15000,
4+
BACKOFF_MULTIPLIER: 1.3,
55
MAX_DURATION_MS: 60 * 60 * 1000,
66
} as const;
77

‎src/upload.ts‎

Lines changed: 80 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -48,16 +48,43 @@ export default class Upload {
4848
time: 100, // Emit progress every 100ms
4949
});
5050

51-
let lastPercent = 0;
51+
const interactive = utils.isInteractive();
52+
let lastPercent = -1;
53+
let lastBucket = -1;
54+
const BUCKET_SIZE = 10;
5255

5356
if (showProgress) {
54-
this.drawProgressBar(fileName, totalSize, 0);
57+
if (interactive) {
58+
this.drawProgressBar(fileName, totalSize, 0);
59+
}
5560

5661
progressTracker.on('progress', (prog) => {
5762
const percent = Math.round(prog.percentage);
58-
if (percent !== lastPercent) {
59-
lastPercent = percent;
60-
this.drawProgressBar(fileName, totalSize, percent);
63+
if (interactive) {
64+
// Redraw whenever percent changes; speed/ETA still update because
65+
// progress-stream emits every 100ms.
66+
if (percent !== lastPercent) {
67+
lastPercent = percent;
68+
this.drawProgressBar(
69+
fileName,
70+
totalSize,
71+
percent,
72+
prog.speed,
73+
prog.eta,
74+
);
75+
}
76+
} else {
77+
// Non-TTY: one line per 10% bucket, no ANSI, no \r. 100% is
78+
// reported by the success path.
79+
const bucket = Math.floor(percent / BUCKET_SIZE);
80+
if (bucket !== lastBucket && percent < 100) {
81+
lastBucket = bucket;
82+
const transferred = this.formatFileSize(
83+
(percent / 100) * totalSize,
84+
);
85+
const total = this.formatFileSize(totalSize);
86+
console.log(` ${fileName}: ${percent}% (${transferred}/${total})`);
87+
}
6188
}
6289
});
6390
}
@@ -98,21 +125,35 @@ export default class Upload {
98125
const result = response.data;
99126
if (result.id) {
100127
if (showProgress) {
101-
this.drawProgressBar(fileName, totalSize, 100);
102-
console.log('');
128+
if (interactive) {
129+
this.drawProgressBar(fileName, totalSize, 100);
130+
console.log('');
131+
} else {
132+
console.log(
133+
` ${fileName}: done (${this.formatFileSize(totalSize)})`,
134+
);
135+
}
103136
}
104137
return { id: result.id };
105138
} else {
106139
if (showProgress) {
107-
console.log(' Failed');
140+
if (interactive) {
141+
console.log(' Failed');
142+
} else {
143+
console.log(` ${fileName}: failed`);
144+
}
108145
}
109146
throw new TestingBotError(
110147
`Upload failed: ${result.error || 'Unknown error'}`,
111148
);
112149
}
113150
} catch (error) {
114151
if (showProgress) {
115-
console.log(' Failed');
152+
if (interactive) {
153+
console.log(' Failed');
154+
} else {
155+
console.log(` ${fileName}: failed`);
156+
}
116157
}
117158
if (error instanceof TestingBotError) {
118159
throw error;
@@ -139,6 +180,8 @@ export default class Upload {
139180
fileName: string,
140181
totalBytes: number,
141182
percent: number,
183+
bytesPerSec?: number,
184+
etaSeconds?: number,
142185
): void {
143186
const barWidth = 30;
144187
const filled = Math.round((barWidth * percent) / 100);
@@ -149,8 +192,24 @@ export default class Upload {
149192
const transferred = this.formatFileSize(transferredBytes);
150193
const total = this.formatFileSize(totalBytes);
151194

195+
let suffix = '';
196+
if (
197+
typeof bytesPerSec === 'number' &&
198+
Number.isFinite(bytesPerSec) &&
199+
bytesPerSec > 0
200+
) {
201+
suffix += ` • ${this.formatFileSize(bytesPerSec)}/s`;
202+
}
203+
if (
204+
typeof etaSeconds === 'number' &&
205+
Number.isFinite(etaSeconds) &&
206+
etaSeconds > 0
207+
) {
208+
suffix += ` • ETA ${this.formatDuration(etaSeconds)}`;
209+
}
210+
152211
process.stdout.write(
153-
`\r ${fileName}: [${bar}] ${percent}% (${transferred}/${total})`,
212+
`\r ${fileName}: [${bar}] ${percent}% (${transferred}/${total})${suffix}`,
154213
);
155214
}
156215

@@ -159,14 +218,24 @@ export default class Upload {
159218
*/
160219
private formatFileSize(bytes: number): string {
161220
if (bytes < 1024) {
162-
return `${bytes} B`;
221+
return `${Math.round(bytes)} B`;
163222
} else if (bytes < 1024 * 1024) {
164223
return `${(bytes / 1024).toFixed(1)} KB`;
165224
} else {
166225
return `${(bytes / (1024 * 1024)).toFixed(2)} MB`;
167226
}
168227
}
169228

229+
/**
230+
* Format seconds as compact duration: "12s" or "3m04s".
231+
*/
232+
private formatDuration(seconds: number): string {
233+
if (seconds < 60) return `${Math.round(seconds)}s`;
234+
const m = Math.floor(seconds / 60);
235+
const s = Math.round(seconds % 60);
236+
return `${m}m${s.toString().padStart(2, '0')}s`;
237+
}
238+
170239
private async validateFile(filePath: string): Promise<void> {
171240
try {
172241
await fs.promises.access(filePath, fs.constants.R_OK);

‎src/utils.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,16 @@ export default {
1010
return `TestingBot-CTL-${packageJson.version}`;
1111
},
1212

13+
/**
14+
* True when stdout is attached to an interactive terminal. CI environments
15+
* (CI=truthy) are always treated as non-interactive so progress animations
16+
* don't spam log aggregators.
17+
*/
18+
isInteractive(): boolean {
19+
if (process.env.CI) return false;
20+
return Boolean(process.stdout.isTTY);
21+
},
22+
1323
getCurrentVersion(): string {
1424
return packageJson.version;
1525
},

‎tests/cli.test.ts‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -627,6 +627,64 @@ describe('TestingBotCTL CLI', () => {
627627
);
628628
});
629629

630+
test('espresso command should throw explicit error when app arg is missing', async () => {
631+
const mockError = jest.fn();
632+
logger.error = mockError;
633+
634+
await program.parseAsync(['node', 'cli', 'espresso']);
635+
636+
expect(mockError).toHaveBeenCalledWith(
637+
expect.stringContaining('Missing required argument:'),
638+
);
639+
expect(mockError).toHaveBeenCalledWith(expect.stringContaining('--app'));
640+
expect(mockError).toHaveBeenCalledWith(
641+
expect.stringContaining('--test-app'),
642+
);
643+
expect(Espresso.prototype.run).not.toHaveBeenCalled();
644+
});
645+
646+
test('maestro command should throw explicit error when app arg is missing', async () => {
647+
const mockError = jest.fn();
648+
logger.error = mockError;
649+
650+
await program.parseAsync(['node', 'cli', 'maestro']);
651+
652+
expect(mockError).toHaveBeenCalledWith(
653+
expect.stringContaining('Missing required argument:'),
654+
);
655+
expect(Maestro.prototype.run).not.toHaveBeenCalled();
656+
});
657+
658+
test('xcuitest command should throw explicit error when app arg is missing', async () => {
659+
const mockError = jest.fn();
660+
logger.error = mockError;
661+
662+
await program.parseAsync(['node', 'cli', 'xcuitest']);
663+
664+
expect(mockError).toHaveBeenCalledWith(
665+
expect.stringContaining('Missing required argument:'),
666+
);
667+
expect(XCUITest.prototype.run).not.toHaveBeenCalled();
668+
});
669+
670+
test('espresso command should not construct provider when credentials are missing', async () => {
671+
mockGetCredentials.mockResolvedValue(null);
672+
const mockError = jest.fn();
673+
logger.error = mockError;
674+
675+
await program.parseAsync([
676+
'node',
677+
'cli',
678+
'espresso',
679+
'app.apk',
680+
'test-app.apk',
681+
]);
682+
683+
// Preflight: run() must not fire if credentials are unresolved,
684+
// even though all required args are present.
685+
expect(Espresso.prototype.run).not.toHaveBeenCalled();
686+
});
687+
630688
test('unknown command should show help', async () => {
631689
const exitSpy = jest
632690
.spyOn(process, 'exit')

‎tests/providers/base_provider.test.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,13 +53,13 @@ describe('BaseProvider.computeNextPollInterval', () => {
5353
});
5454

5555
it('applies backoff when status is unchanged', () => {
56-
expect(provider.computeInterval(5000, false)).toBe(7500);
57-
expect(provider.computeInterval(10000, false)).toBe(15000);
56+
expect(provider.computeInterval(5000, false)).toBe(6500);
57+
expect(provider.computeInterval(10000, false)).toBe(13000);
5858
});
5959

6060
it('caps at the maximum poll interval', () => {
61-
expect(provider.computeInterval(25000, false)).toBe(30000);
62-
expect(provider.computeInterval(100000, false)).toBe(30000);
61+
expect(provider.computeInterval(12000, false)).toBe(15000);
62+
expect(provider.computeInterval(100000, false)).toBe(15000);
6363
});
6464
});
6565

‎tests/providers/espresso.test.ts‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import fs from 'node:fs';
55
import axios, { AxiosError } from 'axios';
66
import { Readable } from 'node:stream';
77
import Credentials from '../../src/models/credentials';
8+
import logger from '../../src/logger';
89

910
jest.mock('axios');
1011
jest.mock('testingbot-tunnel-launcher', () => ({
@@ -18,6 +19,7 @@ jest.mock('../../src/utils', () => ({
1819
getCurrentVersion: jest.fn().mockReturnValue('1.0.0'),
1920
compareVersions: jest.fn().mockReturnValue(0),
2021
checkForUpdate: jest.fn(),
22+
isInteractive: jest.fn().mockReturnValue(true),
2123
},
2224
}));
2325

@@ -979,6 +981,60 @@ describe('Espresso', () => {
979981
expect(espresso['socket']).toBeNull();
980982
});
981983

984+
it('should warn once on connect_error and fall back to polling', () => {
985+
espresso['updateServer'] = 'https://hub.testingbot.com:3031';
986+
espresso['updateKey'] = 'espresso_1234';
987+
988+
let connectErrorHandler: (err: Error) => void = () => {};
989+
mockSocket.on.mockImplementation(
990+
(event: string, handler: (err: Error) => void) => {
991+
if (event === 'connect_error') {
992+
connectErrorHandler = handler;
993+
}
994+
},
995+
);
996+
997+
const warnSpy = jest.spyOn(logger, 'warn').mockImplementation(() => {});
998+
const debugSpy = jest.spyOn(logger, 'debug').mockImplementation(() => {});
999+
1000+
espresso['connectToUpdateServer']();
1001+
connectErrorHandler(new Error('boom'));
1002+
connectErrorHandler(new Error('boom again'));
1003+
1004+
expect(warnSpy).toHaveBeenCalledTimes(1);
1005+
expect(warnSpy).toHaveBeenCalledWith(
1006+
expect.stringContaining('falling back to polling'),
1007+
);
1008+
expect(debugSpy).toHaveBeenCalledWith(expect.stringContaining('boom'));
1009+
1010+
warnSpy.mockRestore();
1011+
debugSpy.mockRestore();
1012+
});
1013+
1014+
it('should not attempt socket connection when quiet is set', () => {
1015+
const quietOptions = new EspressoOptions(
1016+
'path/to/app.apk',
1017+
'path/to/testApp.apk',
1018+
'Pixel 6',
1019+
{ quiet: true },
1020+
);
1021+
const quietEspresso = new Espresso(mockCredentials, quietOptions);
1022+
quietEspresso['updateServer'] = 'https://hub.testingbot.com:3031';
1023+
quietEspresso['updateKey'] = 'espresso_1234';
1024+
1025+
const { io } = jest.requireMock('socket.io-client');
1026+
(io as jest.Mock).mockClear();
1027+
1028+
const warnSpy = jest.spyOn(logger, 'warn').mockImplementation(() => {});
1029+
1030+
quietEspresso['connectToUpdateServer']();
1031+
1032+
expect(io).not.toHaveBeenCalled();
1033+
expect(warnSpy).not.toHaveBeenCalled();
1034+
1035+
warnSpy.mockRestore();
1036+
});
1037+
9821038
it('should handle espresso_data message and write to stdout', () => {
9831039
const stdoutSpy = jest
9841040
.spyOn(process.stdout, 'write')

‎tests/providers/maestro.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ jest.mock('../../src/utils', () => ({
3535
getCurrentVersion: jest.fn().mockReturnValue('1.0.0'),
3636
compareVersions: jest.fn().mockReturnValue(0),
3737
checkForUpdate: jest.fn(),
38+
isInteractive: jest.fn().mockReturnValue(true),
3839
},
3940
}));
4041
jest.mock('glob', () => ({

‎tests/providers/xcuitest.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ jest.mock('../../src/utils', () => ({
1818
getCurrentVersion: jest.fn().mockReturnValue('1.0.0'),
1919
compareVersions: jest.fn().mockReturnValue(0),
2020
checkForUpdate: jest.fn(),
21+
isInteractive: jest.fn().mockReturnValue(true),
2122
},
2223
}));
2324

0 commit comments

Comments
 (0)