mengw15 commented on code in PR #5037:
URL: https://github.com/apache/texera/pull/5037#discussion_r3237887016
##########
frontend/src/app/workspace/component/workspace.component.spec.ts:
##########
@@ -130,12 +130,16 @@ describe("WorkspaceComponent", () => {
routerMock = { navigate: vi.fn() };
locationMock = { go: vi.fn() };
- // TODO(#5015): drop this template override once CodeEditorComponent's
- // own spec is fixed. Real child rendering would let us assert
- // editor-lifecycle wiring; today we stub the host element so the
- // heavyweight children don't compile in the test build.
+ // Replace the component imports with an empty list and rely on
NO_ERRORS_SCHEMA
+ // so the heavyweight children (workflow editor, panels, menu, etc.) become
+ // unknown elements at compile time. The real template still renders, which
+ // means the `<ng-template #codeEditor>` outlet is wired up and the
@ViewChild
+ // query in WorkspaceComponent resolves to a real ViewContainerRef.
Children's
+ // own dependency trees never come into the test build, which keeps the
spec
+ // hermetic without the previous `<div #codeEditor>` template stub. (See
+ // #5015 for why the previous template-override hack existed.)
Review Comment:
Comments should describe what the code means, not what changed. Drop the
"previous <div #codeEditor> template stub" and "#5015 template-override hack
existed" references; describe what the current setup does and why.
##########
frontend/src/app/workspace/component/workspace.component.spec.ts:
##########
@@ -359,4 +372,29 @@ describe("WorkspaceComponent", () => {
expect(component.copilotEnabled).toBe(false);
});
});
+
+ // Real-template rendering coverage — previously the spec replaced the entire
+ // template with `<div #codeEditor>` to keep the heavyweight children out of
+ // the test build, which meant editor lifecycle wiring went untested. With
+ // only the child IMPORTS now stripped (template intact), the real
+ // `<ng-template #codeEditor>` outlet renders and the @ViewChild query
+ // resolves to a live ViewContainerRef. See #5015.
+ describe("child rendering side effects", () => {
+ it("publishes the resolved ViewContainerRef to CodeEditorService.vc on
view init", async () => {
+ await createFixture();
+ // Before view init nothing has been assigned. (The pre-fixture stub on
+ // codeEditorViewRef in createFixture only protects ngOnDestroy teardown
+ // — the service.vc should still be untouched.)
+ expect(codeEditorService.vc).toBeUndefined();
+ fixture.detectChanges();
+ // ngAfterViewInit should hand the live ViewContainerRef from the real
+ // <ng-template #codeEditor> through to CodeEditorService — and not the
+ // pre-test stub. Real ViewContainerRefs expose createEmbeddedView; the
+ // stub does not, so its presence proves the @ViewChild query resolved
+ // against the rendered template rather than the placeholder.
Review Comment:
ditto
##########
frontend/src/app/workspace/component/workspace.component.spec.ts:
##########
@@ -359,4 +372,29 @@ describe("WorkspaceComponent", () => {
expect(component.copilotEnabled).toBe(false);
});
});
+
+ // Real-template rendering coverage — previously the spec replaced the entire
+ // template with `<div #codeEditor>` to keep the heavyweight children out of
+ // the test build, which meant editor lifecycle wiring went untested. With
+ // only the child IMPORTS now stripped (template intact), the real
+ // `<ng-template #codeEditor>` outlet renders and the @ViewChild query
+ // resolves to a live ViewContainerRef. See #5015.
Review Comment:
ditto
##########
frontend/src/app/workspace/component/workspace.component.spec.ts:
##########
@@ -359,4 +372,29 @@ describe("WorkspaceComponent", () => {
expect(component.copilotEnabled).toBe(false);
});
});
+
+ // Real-template rendering coverage — previously the spec replaced the entire
+ // template with `<div #codeEditor>` to keep the heavyweight children out of
+ // the test build, which meant editor lifecycle wiring went untested. With
+ // only the child IMPORTS now stripped (template intact), the real
+ // `<ng-template #codeEditor>` outlet renders and the @ViewChild query
+ // resolves to a live ViewContainerRef. See #5015.
+ describe("child rendering side effects", () => {
+ it("publishes the resolved ViewContainerRef to CodeEditorService.vc on
view init", async () => {
+ await createFixture();
+ // Before view init nothing has been assigned. (The pre-fixture stub on
+ // codeEditorViewRef in createFixture only protects ngOnDestroy teardown
+ // — the service.vc should still be untouched.)
+ expect(codeEditorService.vc).toBeUndefined();
+ fixture.detectChanges();
+ // ngAfterViewInit should hand the live ViewContainerRef from the real
+ // <ng-template #codeEditor> through to CodeEditorService — and not the
+ // pre-test stub. Real ViewContainerRefs expose createEmbeddedView; the
+ // stub does not, so its presence proves the @ViewChild query resolved
+ // against the rendered template rather than the placeholder.
+ expect(codeEditorService.vc).toBeDefined();
Review Comment:
is this line redundant?
--
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]
