Copilot commented on code in PR #7333:
URL: https://github.com/apache/texera/pull/7333#discussion_r3720166193
##########
frontend/src/app/workspace/component/result-panel/console-frame/console-frame.component.spec.ts:
##########
@@ -289,4 +290,91 @@ describe("ConsoleFrameComponent", () => {
expect(component.consoleMessages).toEqual([consoleMessage("PRINT")]);
});
});
+
+ // The tests above drive the class directly; these render the template so its
+ // *ngFor / *ngIf / (click) / [(ngModel)] branches actually execute.
+ describe("template rendering", () => {
+ // A message with a body (renders the collapse panel) that carries a
worker id,
+ // and one with an empty body (renders the plain title branch) and no
worker.
+ const withBody: ConsoleMessage = {
+ ...consoleMessage("PRINT"),
+ message: "hello body",
+ title: "header A",
+ workerId: "w-0",
+ source: "srcA",
+ };
+ const noBody: ConsoleMessage = {
+ ...consoleMessage("ERROR"),
+ message: "",
+ title: "plain B",
+ workerId: "",
+ source: "srcB",
+ };
+
+ it("renders one row per message with its body, source, timestamp and
worker tags", () => {
+ component.consoleMessages = [withBody, noBody];
+ component.showSource = true;
+ component.showTimestamp = true;
+ fixture.detectChanges();
+
+ const rows =
fixture.debugElement.queryAll(By.css(".console-message-entry"));
+ expect(rows.length).toBe(2);
+
+ // non-empty message -> collapse header; empty message -> plain title
+ const text = fixture.nativeElement.textContent as string;
+
expect(fixture.debugElement.query(By.css(".collapse-message-header")).nativeElement.textContent).toContain(
+ "header A"
+ );
+ expect(text).toContain("plain B");
+
+ // both rows show a source tag; both show a timestamp tag (the rendered
date
+ // string is intentionally NOT asserted — it is timezone-dependent)
+
expect(fixture.debugElement.queryAll(By.css(".source-tag")).length).toBe(2);
+
expect(fixture.debugElement.queryAll(By.css(".timestamp-tag")).length).toBe(2);
+ // only the message with a worker id renders the worker tag
+
expect(fixture.debugElement.queryAll(By.css(".worker-tag")).length).toBe(1);
+ });
+
+ it("hides the source and timestamp tags when the toggles are off", () => {
+ component.consoleMessages = [withBody, noBody];
+ component.showSource = false;
+ component.showTimestamp = false;
+ fixture.detectChanges();
+
+
expect(fixture.debugElement.queryAll(By.css(".console-message-entry")).length).toBe(2);
+
expect(fixture.debugElement.queryAll(By.css(".source-tag")).length).toBe(0);
+
expect(fixture.debugElement.queryAll(By.css(".timestamp-tag")).length).toBe(0);
+ });
+
+ it("does not render the debug input group when console input is disabled",
() => {
+ component.consoleInputEnabled = false;
+ fixture.detectChanges();
+
expect(fixture.debugElement.query(By.css(".console-input-container"))).toBeNull();
+ });
+
+ it("renders the debug input group and wires its buttons and command input
when enabled", () => {
+ component.operatorId = "op1";
+ component.workerIds = ["w-0", "w-1"];
+ component.targetWorker = component.ALL_WORKERS;
Review Comment:
The setup sets `targetWorker` to `ALL_WORKERS`, but the assertion expects a
request for a specific workerId (`"w-0"`). This is internally inconsistent and
can make the test pass/fail for the wrong reason depending on component
behavior. Align the test by either (a) setting `component.targetWorker = "w-0"`
before submitting, or (b) updating the expectation(s) to match the
`ALL_WORKERS` semantics (e.g., multiple sends or a sentinel worker value).
##########
frontend/src/app/workspace/component/result-panel/console-frame/console-frame.component.spec.ts:
##########
@@ -289,4 +290,91 @@ describe("ConsoleFrameComponent", () => {
expect(component.consoleMessages).toEqual([consoleMessage("PRINT")]);
});
});
+
+ // The tests above drive the class directly; these render the template so its
+ // *ngFor / *ngIf / (click) / [(ngModel)] branches actually execute.
+ describe("template rendering", () => {
+ // A message with a body (renders the collapse panel) that carries a
worker id,
+ // and one with an empty body (renders the plain title branch) and no
worker.
+ const withBody: ConsoleMessage = {
+ ...consoleMessage("PRINT"),
+ message: "hello body",
+ title: "header A",
+ workerId: "w-0",
+ source: "srcA",
+ };
+ const noBody: ConsoleMessage = {
+ ...consoleMessage("ERROR"),
+ message: "",
+ title: "plain B",
+ workerId: "",
+ source: "srcB",
+ };
+
+ it("renders one row per message with its body, source, timestamp and
worker tags", () => {
+ component.consoleMessages = [withBody, noBody];
+ component.showSource = true;
+ component.showTimestamp = true;
+ fixture.detectChanges();
+
+ const rows =
fixture.debugElement.queryAll(By.css(".console-message-entry"));
+ expect(rows.length).toBe(2);
+
+ // non-empty message -> collapse header; empty message -> plain title
+ const text = fixture.nativeElement.textContent as string;
+
expect(fixture.debugElement.query(By.css(".collapse-message-header")).nativeElement.textContent).toContain(
+ "header A"
+ );
Review Comment:
`query(By.css(...)).nativeElement` will throw a less-informative error if
the element is missing (e.g., template refactor or CSS class rename). Make the
expectation more robust by first asserting the element exists (store it in a
const, expect it toBeTruthy/not.toBeNull), then assert its text content. This
improves failure diagnostics without changing the intent of the test.
##########
frontend/src/app/workspace/component/result-panel/console-frame/console-frame.component.spec.ts:
##########
@@ -289,4 +290,91 @@ describe("ConsoleFrameComponent", () => {
expect(component.consoleMessages).toEqual([consoleMessage("PRINT")]);
});
});
+
+ // The tests above drive the class directly; these render the template so its
+ // *ngFor / *ngIf / (click) / [(ngModel)] branches actually execute.
+ describe("template rendering", () => {
+ // A message with a body (renders the collapse panel) that carries a
worker id,
+ // and one with an empty body (renders the plain title branch) and no
worker.
+ const withBody: ConsoleMessage = {
+ ...consoleMessage("PRINT"),
+ message: "hello body",
+ title: "header A",
+ workerId: "w-0",
+ source: "srcA",
+ };
+ const noBody: ConsoleMessage = {
+ ...consoleMessage("ERROR"),
+ message: "",
+ title: "plain B",
+ workerId: "",
+ source: "srcB",
+ };
+
+ it("renders one row per message with its body, source, timestamp and
worker tags", () => {
+ component.consoleMessages = [withBody, noBody];
+ component.showSource = true;
+ component.showTimestamp = true;
+ fixture.detectChanges();
+
+ const rows =
fixture.debugElement.queryAll(By.css(".console-message-entry"));
+ expect(rows.length).toBe(2);
+
+ // non-empty message -> collapse header; empty message -> plain title
+ const text = fixture.nativeElement.textContent as string;
+
expect(fixture.debugElement.query(By.css(".collapse-message-header")).nativeElement.textContent).toContain(
+ "header A"
+ );
+ expect(text).toContain("plain B");
+
+ // both rows show a source tag; both show a timestamp tag (the rendered
date
+ // string is intentionally NOT asserted — it is timezone-dependent)
+
expect(fixture.debugElement.queryAll(By.css(".source-tag")).length).toBe(2);
+
expect(fixture.debugElement.queryAll(By.css(".timestamp-tag")).length).toBe(2);
+ // only the message with a worker id renders the worker tag
+
expect(fixture.debugElement.queryAll(By.css(".worker-tag")).length).toBe(1);
+ });
+
+ it("hides the source and timestamp tags when the toggles are off", () => {
+ component.consoleMessages = [withBody, noBody];
+ component.showSource = false;
+ component.showTimestamp = false;
+ fixture.detectChanges();
+
+
expect(fixture.debugElement.queryAll(By.css(".console-message-entry")).length).toBe(2);
+
expect(fixture.debugElement.queryAll(By.css(".source-tag")).length).toBe(0);
+
expect(fixture.debugElement.queryAll(By.css(".timestamp-tag")).length).toBe(0);
+ });
+
+ it("does not render the debug input group when console input is disabled",
() => {
+ component.consoleInputEnabled = false;
+ fixture.detectChanges();
+
expect(fixture.debugElement.query(By.css(".console-input-container"))).toBeNull();
+ });
+
+ it("renders the debug input group and wires its buttons and command input
when enabled", () => {
+ component.operatorId = "op1";
+ component.workerIds = ["w-0", "w-1"];
+ component.targetWorker = component.ALL_WORKERS;
+ component.consoleInputEnabled = true;
+ fixture.detectChanges();
+
+
expect(fixture.debugElement.query(By.css(".console-input-container"))).toBeTruthy();
+
+ // clicking each action button reaches its handler / service
+ const buttons =
fixture.debugElement.queryAll(By.css(".console-input-container button"));
+ expect(buttons.length).toBe(4);
+ buttons.forEach(button => button.triggerEventHandler("click", {}));
+ expect(skipTuples).toHaveBeenCalled();
+ expect(retryExecution).toHaveBeenCalled();
+ expect(doStep).toHaveBeenCalled();
+ expect(doContinue).toHaveBeenCalled();
+
+ // entering a command and pressing enter submits it through the websocket
+ // (target input[nz-input] specifically — the nz-select renders its own
input too)
+ component.command = "break";
+
fixture.debugElement.query(By.css("input[nz-input]")).triggerEventHandler("keyup.enter",
{});
+ expect(send).toHaveBeenCalledWith("DebugCommandRequest", { operatorId:
"op1", workerId: "w-0", cmd: "break" });
Review Comment:
The setup sets `targetWorker` to `ALL_WORKERS`, but the assertion expects a
request for a specific workerId (`"w-0"`). This is internally inconsistent and
can make the test pass/fail for the wrong reason depending on component
behavior. Align the test by either (a) setting `component.targetWorker = "w-0"`
before submitting, or (b) updating the expectation(s) to match the
`ALL_WORKERS` semantics (e.g., multiple sends or a sentinel worker value).
##########
frontend/src/app/workspace/component/result-panel/console-frame/console-frame.component.spec.ts:
##########
@@ -289,4 +290,91 @@ describe("ConsoleFrameComponent", () => {
expect(component.consoleMessages).toEqual([consoleMessage("PRINT")]);
});
});
+
+ // The tests above drive the class directly; these render the template so its
+ // *ngFor / *ngIf / (click) / [(ngModel)] branches actually execute.
+ describe("template rendering", () => {
+ // A message with a body (renders the collapse panel) that carries a
worker id,
+ // and one with an empty body (renders the plain title branch) and no
worker.
+ const withBody: ConsoleMessage = {
+ ...consoleMessage("PRINT"),
+ message: "hello body",
+ title: "header A",
+ workerId: "w-0",
+ source: "srcA",
+ };
+ const noBody: ConsoleMessage = {
+ ...consoleMessage("ERROR"),
+ message: "",
+ title: "plain B",
+ workerId: "",
+ source: "srcB",
+ };
+
+ it("renders one row per message with its body, source, timestamp and
worker tags", () => {
+ component.consoleMessages = [withBody, noBody];
+ component.showSource = true;
+ component.showTimestamp = true;
+ fixture.detectChanges();
+
+ const rows =
fixture.debugElement.queryAll(By.css(".console-message-entry"));
+ expect(rows.length).toBe(2);
+
+ // non-empty message -> collapse header; empty message -> plain title
+ const text = fixture.nativeElement.textContent as string;
+
expect(fixture.debugElement.query(By.css(".collapse-message-header")).nativeElement.textContent).toContain(
+ "header A"
+ );
+ expect(text).toContain("plain B");
+
+ // both rows show a source tag; both show a timestamp tag (the rendered
date
+ // string is intentionally NOT asserted — it is timezone-dependent)
+
expect(fixture.debugElement.queryAll(By.css(".source-tag")).length).toBe(2);
+
expect(fixture.debugElement.queryAll(By.css(".timestamp-tag")).length).toBe(2);
+ // only the message with a worker id renders the worker tag
+
expect(fixture.debugElement.queryAll(By.css(".worker-tag")).length).toBe(1);
+ });
+
+ it("hides the source and timestamp tags when the toggles are off", () => {
+ component.consoleMessages = [withBody, noBody];
+ component.showSource = false;
+ component.showTimestamp = false;
+ fixture.detectChanges();
+
+
expect(fixture.debugElement.queryAll(By.css(".console-message-entry")).length).toBe(2);
+
expect(fixture.debugElement.queryAll(By.css(".source-tag")).length).toBe(0);
+
expect(fixture.debugElement.queryAll(By.css(".timestamp-tag")).length).toBe(0);
+ });
+
+ it("does not render the debug input group when console input is disabled",
() => {
+ component.consoleInputEnabled = false;
+ fixture.detectChanges();
+
expect(fixture.debugElement.query(By.css(".console-input-container"))).toBeNull();
+ });
+
+ it("renders the debug input group and wires its buttons and command input
when enabled", () => {
+ component.operatorId = "op1";
+ component.workerIds = ["w-0", "w-1"];
+ component.targetWorker = component.ALL_WORKERS;
+ component.consoleInputEnabled = true;
+ fixture.detectChanges();
+
+
expect(fixture.debugElement.query(By.css(".console-input-container"))).toBeTruthy();
+
+ // clicking each action button reaches its handler / service
+ const buttons =
fixture.debugElement.queryAll(By.css(".console-input-container button"));
+ expect(buttons.length).toBe(4);
+ buttons.forEach(button => button.triggerEventHandler("click", {}));
Review Comment:
Passing `{}` as the click event can mask issues if handlers access event
properties (e.g., `preventDefault`, `stopPropagation`, or mouse coordinates).
Prefer passing a real `MouseEvent` (or `null` if the handler doesn’t use the
event) to better reflect actual runtime behavior and reduce the chance of false
positives.
--
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]
