Skip to content

chore(e2e-tests): force wait for initialization before writing input #2600

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+41 −18
Open

chore(e2e-tests): force wait for initialization before writing input #2600

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions packages/e2e-tests/test/e2e-direct.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,10 @@ describe('e2e direct connection', function () {
const shell = this.startTestShell({
args: [await rs0.connectionString()],
});
await shell.waitForPrompt();
await shell.executeLine(`db.getSiblingDB("${dbname}").dropDatabase()`);
shell.writeInputLine('exit');
await shell.waitForSuccessfulExit();
});

context('connecting to secondary members directly', function () {
Expand Down
14 changes: 5 additions & 9 deletions packages/e2e-tests/test/e2e.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,10 +310,9 @@ describe('e2e', function () {
// The number of newlines here matters
shell.writeInput(
'sleep(100);print([1,2,3,4,5,6,7,8,9,10].reduce(\n(a,b) => { return a*b; }, 1))\n\n\n\n',
{ end: true }
{ end: true, requireFinishedInitialization: false }
);
await shell.waitForSuccessfulExit();
shell.assertContainsOutput('3628800');
expect(await shell.waitForCleanOutput()).to.include('3628800');
});
it('ignores control characters in TTY input', async function () {
shell = this.startTestShell({
Expand Down Expand Up @@ -1155,10 +1154,9 @@ describe('e2e', function () {
});
shell.writeInput(
'[db.hello()].reduce(\n() => { return 11111*11111; },0)\n\n\n',
{ end: true }
{ end: true, requireFinishedInitialization: false }
);
await shell.waitForSuccessfulExit();
shell.assertContainsOutput('123454321');
expect(await shell.waitForCleanOutput()).to.include('123454321');
});
});

Expand Down Expand Up @@ -2605,9 +2603,7 @@ describe('e2e', function () {

it('allows connecting to a host and running commands against it', async function () {
const connectionString = await testServer.connectionString();
await eventually(() => {
shell.assertContainsOutput('Please enter a MongoDB connection string');
});
await shell.waitForLine(/Please enter a MongoDB connection string/);
shell.writeInputLine(connectionString);
await shell.waitForPrompt();

Expand Down
43 changes: 34 additions & 9 deletions packages/e2e-tests/test/test-shell.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,16 @@ type SignalType = ChildProcess extends { kill: (signal: infer T) => any }
? T
: never;

export interface TestShellInputOptions {
end?: boolean;
requireFinishedInitialization?: boolean;
}

export interface TestShellWaitForPromptOptions {
timeout?: number;
promptPattern?: RegExp;
}

// Assume that prompt strings are those that end in '> ' but do not contain
// < or > (so that e.g. '- <repl>' in a stack trace is not considered a prompt).
const PROMPT_PATTERN = /^([^<>]*> ?)+$/m;
Expand Down Expand Up @@ -152,6 +162,7 @@ export class TestShell {
private _output: string;
private _rawOutput: string;
private _onClose: Promise<number>;
private _initializationKnownToBeFinished = false;

constructor(
shellProcess: ChildProcessWithoutNullStreams,
Expand Down Expand Up @@ -205,11 +216,14 @@ export class TestShell {
});
}
});
// Not technically true, but in practice the patterns we wait for
// are sufficient to indicate that initialization is done.
this._initializationKnownToBeFinished = true;
}

async waitForPrompt(
start = 0,
opts: { timeout?: number; promptPattern?: RegExp } = {}
opts: TestShellWaitForPromptOptions = {}
): Promise<void> {
await eventually(
() => {
Expand Down Expand Up @@ -247,10 +261,13 @@ export class TestShell {
},
{ ...opts }
);
this._initializationKnownToBeFinished = true;
}

waitForAnyExit(): Promise<number> {
return this._onClose;
async waitForAnyExit(): Promise<number> {
const code = await this._onClose;
this._initializationKnownToBeFinished = true;
return code;
}

async waitForSuccessfulExit(): Promise<void> {
Expand All @@ -267,7 +284,7 @@ export class TestShell {
}

async waitForPromptOrExit(
opts: { timeout?: number; start?: number } = {}
opts: TestShellWaitForPromptOptions & { start?: number } = {}
): Promise<TestShellStartupResult> {
return Promise.race([
this.waitForPrompt(opts.start ?? 0, opts).then(
Expand All @@ -283,21 +300,29 @@ export class TestShell {
this._process.kill(signal);
}

writeInput(chars: string, { end = false } = {}): void {
writeInput(
chars: string,
{
end = false,
requireFinishedInitialization = true,
}: TestShellInputOptions = {}
): void {
if (requireFinishedInitialization && !this._initializationKnownToBeFinished)
throw new Error('Wait for shell to be initialized before writing input');
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message should be more actionable. Consider: 'Cannot write input before shell initialization completes. Call waitForPrompt(), waitForLine(), or waitForAnyExit() first, or set requireFinishedInitialization to false.'

Suggested change
throw new Error('Wait for shell to be initialized before writing input');
throw new Error(
"Cannot write input before shell initialization completes. Call waitForPrompt(), waitForLine(), or waitForAnyExit() first, or set requireFinishedInitialization to false."
);

Copilot uses AI. Check for mistakes.
Copy link
Collaborator Author

@addaleax addaleax Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any strong feelings? I imagine anybody getting this error would look up the logic where it's being thrown (i.e. the code), so I'd personally am okay with leaning towards brevity, but I also don't care too deeply

Copy link
Contributor

@gagik gagik Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah don't mind either way either.

this._process.stdin.write(chars);
if (end) this._process.stdin.end();
}

writeInputLine(chars: string): void {
this.writeInput(`${chars}\n`);
writeInputLine(chars: string, options?: TestShellInputOptions): void {
this.writeInput(`${chars}\n`, options);
}

async executeLine(
line: string,
opts: { timeout?: number; promptPattern?: RegExp } = {}
opts: TestShellWaitForPromptOptions & TestShellInputOptions = {}
): Promise<string> {
const previousOutputLength = this._output.length;
this.writeInputLine(line);
this.writeInputLine(line, opts);
await this.waitForPrompt(previousOutputLength, opts);
return this._output.slice(previousOutputLength);
}
Expand Down
Loading