Skip to content

Commit 5a6af76

Browse files
OrKoNDevtools-frontend LUCI CQ
authored andcommitted
[AI Assistance] Remove time-based assertions in unit tests
Drive-by: a helper to create cancelled results with an error and an extra check for the signal before triggering the side effect UI. Bug: 380394373 Change-Id: I93e805d86db663f9450a35e2a85396331b5d004a Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6108440 Reviewed-by: Mathias Bynens <[email protected]> Commit-Queue: Alex Rudenko <[email protected]>
1 parent 2ab963d commit 5a6af76

File tree

2 files changed

+37
-30
lines changed

2 files changed

+37
-30
lines changed

front_end/panels/ai_assistance/agents/StylingAgent.test.ts

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -712,8 +712,7 @@ c`;
712712
});
713713

714714
it('returns error when side effect is aborted', async () => {
715-
const promise = Promise.withResolvers();
716-
const stub = sinon.stub().returns(promise);
715+
const selected = new AiAssistance.NodeContext(element);
717716
async function* generateAction() {
718717
yield {
719718
explanation: `ACTION
@@ -725,26 +724,31 @@ c`;
725724
}
726725
const execJs = sinon.mock().once().throws(
727726
new AiAssistance.SideEffectError('EvalError: Possible side-effect in debug-evaluate'));
727+
const sideEffectConfirmationPromise = Promise.withResolvers();
728728
const agent = new StylingAgent({
729729
aidaClient: mockAidaClient(generateAction),
730730
createExtensionScope,
731-
confirmSideEffectForTest: stub,
731+
confirmSideEffectForTest: sinon.stub().returns(sideEffectConfirmationPromise),
732732
execJs,
733733
});
734-
const controller = new AbortController();
735734

736-
const agentPromise = Array.fromAsync(
737-
agent.run('test', {selected: new AiAssistance.NodeContext(element), signal: controller.signal}));
738-
await stub.calledOnce;
739-
const awaitTimeout = (delay: number) => new Promise(resolve => setTimeout(resolve, delay));
740-
await awaitTimeout(10).then(() => controller.abort());
741-
const responses = await agentPromise;
735+
const responses: AiAssistance.ResponseData[] = [];
736+
const controller = new AbortController();
737+
for await (const result of agent.run('test', {selected, signal: controller.signal})) {
738+
responses.push(result);
739+
if (result.type === 'side-effect') {
740+
// Initial code invocation resulting in a side-effect
741+
// happened.
742+
assert.isTrue(execJs.calledOnce);
743+
// Emulate abort when waiting for the side-effect confirmation.
744+
controller.abort();
745+
}
746+
}
742747

743748
const errorStep = responses.at(-1) as AiAssistance.ErrorResponse;
744-
assert.strictEqual(execJs.getCalls().length, 1);
745749
assert.exists(errorStep);
746750
assert.strictEqual(errorStep.error, ErrorType.ABORT);
747-
await promise.promise.then(value => assert.strictEqual(value, false));
751+
assert.strictEqual(await sideEffectConfirmationPromise.promise, false);
748752
});
749753
});
750754

front_end/panels/ai_assistance/agents/StylingAgent.ts

Lines changed: 21 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -586,24 +586,25 @@ export class StylingAgent extends AiAgent<SDK.DOMModel.DOMNode> {
586586
handleAction(action: string, options?: {signal?: AbortSignal}):
587587
AsyncGenerator<SideEffectResponse, ActionResponse, void> {
588588
debugLog(`Action to execute: ${action}`);
589-
if (this.executionMode === Root.Runtime.HostConfigFreestylerExecutionMode.NO_SCRIPTS) {
589+
590+
function createCancelledResult(output: string): ActionResponse {
590591
return {
591592
type: ResponseType.ACTION,
592593
code: action,
593-
output: 'Error: JavaScript execution is currently disabled.',
594+
output,
594595
canceled: true,
595596
};
596597
}
597598

599+
if (this.executionMode === Root.Runtime.HostConfigFreestylerExecutionMode.NO_SCRIPTS) {
600+
return createCancelledResult('Error: JavaScript execution is currently disabled.');
601+
}
602+
598603
const selectedNode = UI.Context.Context.instance().flavor(SDK.DOMModel.DOMNode);
599604
const target = selectedNode?.domModel().target() ?? UI.Context.Context.instance().flavor(SDK.Target.Target);
600605
if (target?.model(SDK.DebuggerModel.DebuggerModel)?.selectedCallFrame()) {
601-
return {
602-
type: ResponseType.ACTION,
603-
code: action,
604-
output: 'Error: Cannot evaluate JavaScript because the execution is paused on a breakpoint.',
605-
canceled: true,
606-
};
606+
return createCancelledResult(
607+
'Error: Cannot evaluate JavaScript because the execution is paused on a breakpoint.');
607608
}
608609

609610
const scope = this.#createExtensionScope(this.#changes);
@@ -613,16 +614,22 @@ export class StylingAgent extends AiAgent<SDK.DOMModel.DOMNode> {
613614
debugLog(`Action result: ${JSON.stringify(result)}`);
614615
if (result.sideEffect) {
615616
if (this.executionMode === Root.Runtime.HostConfigFreestylerExecutionMode.SIDE_EFFECT_FREE_SCRIPTS_ONLY) {
616-
return {
617-
type: ResponseType.ACTION,
618-
code: action,
619-
output: 'Error: JavaScript execution that modifies the page is currently disabled.',
620-
canceled: true,
621-
};
617+
return createCancelledResult('Error: JavaScript execution that modifies the page is currently disabled.');
618+
}
619+
620+
if (options?.signal?.aborted) {
621+
return createCancelledResult('Error: evaluation has been cancelled');
622622
}
623623

624624
const sideEffectConfirmationPromiseWithResolvers = this.#confirmSideEffect<boolean>();
625625

626+
void sideEffectConfirmationPromiseWithResolvers.promise.then(result => {
627+
Host.userMetrics.actionTaken(
628+
result ? Host.UserMetrics.Action.AiAssistanceSideEffectConfirmed :
629+
Host.UserMetrics.Action.AiAssistanceSideEffectRejected,
630+
);
631+
});
632+
626633
options?.signal?.addEventListener('abort', () => {
627634
sideEffectConfirmationPromiseWithResolvers.resolve(false);
628635
}, {once: true});
@@ -632,10 +639,6 @@ export class StylingAgent extends AiAgent<SDK.DOMModel.DOMNode> {
632639
code: action,
633640
confirm: (result: boolean) => {
634641
sideEffectConfirmationPromiseWithResolvers.resolve(result);
635-
Host.userMetrics.actionTaken(
636-
result ? Host.UserMetrics.Action.AiAssistanceSideEffectConfirmed :
637-
Host.UserMetrics.Action.AiAssistanceSideEffectRejected,
638-
);
639642
},
640643
};
641644

0 commit comments

Comments
 (0)