Copilot commented on code in PR #6603:
URL: https://github.com/apache/texera/pull/6603#discussion_r3613853677
##########
frontend/src/app/workspace/service/undo-redo/undo-redo.service.spec.ts:
##########
@@ -83,4 +84,97 @@ describe("UndoRedoService", () => {
expect(service.getUndoLength()).toEqual(1);
expect(service.getRedoLength()).toEqual(0);
});
+
+ describe("with a stubbed undo manager", () => {
+ let undoRedo: UndoRedoService;
+ let manager: {
+ canUndo: ReturnType<typeof vi.fn>;
+ canRedo: ReturnType<typeof vi.fn>;
+ undo: ReturnType<typeof vi.fn>;
+ redo: ReturnType<typeof vi.fn>;
+ clear: ReturnType<typeof vi.fn>;
+ undoStack: unknown[];
+ redoStack: unknown[];
+ };
+
+ beforeEach(() => {
+ undoRedo = new UndoRedoService();
+ manager = {
+ canUndo: vi.fn().mockReturnValue(true),
+ canRedo: vi.fn().mockReturnValue(true),
+ undo: vi.fn(),
+ redo: vi.fn(),
+ clear: vi.fn(),
+ undoStack: [1, 2, 3],
+ redoStack: [1],
+ };
+ undoRedo.setUndoManager(manager as unknown as Y.UndoManager);
+ });
+
+ it("undoAction / redoAction delegate to the manager and toggle the
joint-command guard back on", () => {
+ undoRedo.undoAction();
+ undoRedo.redoAction();
+
+ expect(manager.undo).toHaveBeenCalledTimes(1);
+ expect(manager.redo).toHaveBeenCalledTimes(1);
+ expect(undoRedo.listenJointCommand).toBe(true);
+ });
+
+ it("undoAction / redoAction are no-ops when the manager cannot undo/redo",
() => {
+ manager.canUndo.mockReturnValue(false);
+ manager.canRedo.mockReturnValue(false);
+
+ undoRedo.undoAction();
+ undoRedo.redoAction();
+
+ expect(manager.undo).not.toHaveBeenCalled();
+ expect(manager.redo).not.toHaveBeenCalled();
+ });
+
+ it("undoAction / redoAction are no-ops while workflow modification is
disabled", () => {
+ const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
+ undoRedo.disableWorkFlowModification();
+
+ undoRedo.undoAction();
+ undoRedo.redoAction();
+
+ expect(manager.undo).not.toHaveBeenCalled();
+ expect(manager.redo).not.toHaveBeenCalled();
+ errorSpy.mockRestore();
+ });
Review Comment:
`errorSpy.mockRestore()` won’t run if an assertion in this test fails, which
can leak the console spy into later tests and cause cascading failures. Use a
`try/finally` (or an `afterEach`) to guarantee restoration.
##########
frontend/src/app/workspace/service/workflow-console/workflow-console.service.spec.ts:
##########
@@ -47,4 +92,21 @@ describe("WorkflowConsoleService", () => {
expect(service.hasConsoleMessages("op1")).toBe(false);
expect(notified).toBe(true);
});
+
+ it("registerAutoClearConsoleMessages clears the store when execution
re-initializes", () => {
Review Comment:
This test name suggests it is exercising
`registerAutoClearConsoleMessages()` directly, but the spec never calls that
method; it only relies on the constructor side-effect. Renaming the test will
make the intent clearer and avoid confusion if the constructor behavior changes
in the future.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
