From 866ff46b1af5f8540f73c3e530d3d7a2c699a9aa Mon Sep 17 00:00:00 2001 From: kimyenac Date: Sat, 29 Aug 2026 13:57:34 +0900 Subject: [PATCH 1/4] [ZEPPELIN-6650] Add unit tests for the React result renderers Covers the two files that decide what a result looks like before it is drawn, per the conventions in projects/zeppelin-react/AGENTS.md. SingleResultRenderer gets one spec per DatasetType arm, asserting what the user ends up seeing rather than which component was chosen: markup for HTML, a base64 png for IMG, the unsupported notice for ANGULAR, and nothing at all for a type with no renderer, since drawing something misleading would be worse. The TABLE arm asserts only that it was taken. What the visualization does with the data is its own concern, and its display-mode state is the known defect AGENTS.md says to fix rather than pin. One more spec covers the config lookup beside it: the entry has to be the one at this result's index, which a fixture configured to draw a chart at index 1 and a table at index 0 makes visible. That leaves the file at every branch covered. HTMLRenderer is the reason that file assigns innerHTML by hand: a script parsed out of innerHTML is inert for good, so the component builds a fresh element and puts it in the old one's place. jsdom never runs scripts, and neither runScripts nor the environment options change that here, so execution itself is out of reach and belongs in e2e. What is observable is the swap, and that is what these specs pin: attributes and body carried over, async forced off so a library cannot arrive after the code using it, and each script left where it was in the markup. The code-block highlighting is covered too, without pinning which language highlight.js guesses. The one branch left is the null container, which React fills before the effect runs. Two things left out on purpose. A carriage-return spec at the SingleResultRenderer level cannot fail, because ansi-to-react already collapses \r the same way checkAndReplaceCarriageReturn does, and textUtils.spec.ts covers the helper itself. An unmount spec cannot fail either, since React removes the container it owns whether or not the effect cleans up. The matchMedia stub in test-setup.ts is what lets the TABLE arm render at all: antd's responsive observer calls it and jsdom implements none. The same block appears in the ZEPPELIN-6630 branch, so whichever of the two lands second drops it in rebase. --- .../renderers/HTMLRenderer.spec.tsx | 82 +++++++++++++++++++ .../templates/SingleResultRenderer.spec.tsx | 75 +++++++++++++++++ 2 files changed, 157 insertions(+) create mode 100644 zeppelin-web-angular/projects/zeppelin-react/src/components/renderers/HTMLRenderer.spec.tsx create mode 100644 zeppelin-web-angular/projects/zeppelin-react/src/templates/SingleResultRenderer.spec.tsx diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/components/renderers/HTMLRenderer.spec.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/components/renderers/HTMLRenderer.spec.tsx new file mode 100644 index 00000000000..fd195971fe3 --- /dev/null +++ b/zeppelin-web-angular/projects/zeppelin-react/src/components/renderers/HTMLRenderer.spec.tsx @@ -0,0 +1,82 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { render, screen, waitFor } from '@testing-library/react'; +import { describe, expect, it } from 'vitest'; +import { HTMLRenderer } from './HTMLRenderer'; + +// jsdom never runs scripts, so the execution this component exists for is out +// of reach here and belongs in e2e. These specs pin the swap that makes it +// possible: a script parsed out of innerHTML is inert, so it is rebuilt. +describe('HTMLRenderer', () => { + it('renders the markup it is given', () => { + render(); + + expect(screen.getByText('rendered output')).toBeTruthy(); + }); + + it('carries the original attributes onto the replacement script', () => { + const { container } = render( + + ); + + const script = container.querySelector('script')!; + expect(script.getAttribute('type')).toBe('text/javascript'); + expect(script.getAttribute('data-mark')).toBe('kept'); + expect(script.getAttribute('src')).toBe('lib.js'); + }); + + it('keeps the script body so the replacement has something to run', () => { + const { container } = render(); + + expect(container.querySelector('script')!.textContent).toBe('window.answer = 42;'); + }); + + it('forces async off even when the source markup asked for it', () => { + // A library and the code using it must not arrive out of order. + const { container } = render(); + + expect(container.querySelector('script')!.async).toBe(false); + }); + + it('leaves each script where it was among the surrounding markup', () => { + const { container } = render( + + ); + + const ids = Array.from(container.querySelectorAll('.inner-html > *')).map(node => node.id || node.tagName); + expect(ids).toEqual(['P', 'one', 'P', 'two']); + }); + + it('highlights a code block, matching what the Angular renderer does', async () => { + const { container } = render(); + + // highlight.js arrives through a dynamic import, so the class lands a tick + // later. Which language it guesses is its own business and not pinned here. + await waitFor(() => expect(container.querySelector('pre code')!.classList.contains('hljs')).toBe(true)); + }); + + it('replaces the previous output when the html changes', () => { + const { rerender } = render(); + + rerender(); + + expect(screen.queryByText('first')).toBeNull(); + expect(screen.getByText('second')).toBeTruthy(); + }); + + it('renders nothing visible for empty html', () => { + const { container } = render(); + + expect(container.textContent).toBe(''); + }); +}); diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/templates/SingleResultRenderer.spec.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/templates/SingleResultRenderer.spec.tsx new file mode 100644 index 00000000000..dc4f60362c8 --- /dev/null +++ b/zeppelin-web-angular/projects/zeppelin-react/src/templates/SingleResultRenderer.spec.tsx @@ -0,0 +1,75 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { render, screen } from '@testing-library/react'; +import { describe, expect, it } from 'vitest'; +import { DatasetType, ParagraphConfigResults, ParagraphIResultsMsgItem } from '@zeppelin/sdk'; +import { SingleResultRenderer } from './SingleResultRenderer'; + +const result = (type: DatasetType, data: string): ParagraphIResultsMsgItem => ({ type, data }); + +const TABLE_DATA = 'name\tage\nalice\t30'; + +// Index 0 stays a table, index 1 draws a chart, so reading the wrong entry shows. +const configs = { + 0: { graph: { mode: 'table' } }, + 1: { graph: { mode: 'multiBarChart' } } +} as unknown as ParagraphConfigResults; + +describe('SingleResultRenderer', () => { + it('renders TEXT as preformatted output', () => { + render(); + + expect(screen.getByText(/line one/)).toBeTruthy(); + }); + + it('renders TABLE through the visualization', () => { + render(); + + // Only that the arm was taken. The visualization's display-mode state is a + // known defect (projects/zeppelin-react/AGENTS.md), so nothing here pins it. + expect(screen.getByText('alice')).toBeTruthy(); + expect(screen.getByRole('button', { name: /Bar Chart/ })).toBeTruthy(); + }); + + it('hands the visualization the display config for its own result index', () => { + render(); + + expect(screen.queryByText('alice')).toBeNull(); + }); + + it('renders IMG as a base64 png', () => { + render(); + + expect(screen.getByRole('img').getAttribute('src')).toBe('data:image/png;base64,QUJD'); + }); + + it('renders HTML as markup rather than as text', () => { + render(markup output

')} />); + + expect(screen.getByText('markup output').tagName).toBe('P'); + }); + + it('tells the user that ANGULAR results are unsupported here', () => { + render(); + + expect(screen.getByText('Angular Component')).toBeTruthy(); + expect(screen.getByText(/not supported in React environment/)).toBeTruthy(); + }); + + it('renders nothing for a type it has no renderer for', () => { + // NETWORK is declared by the SDK and reaches the default arm. + const { container } = render(); + + expect(container.innerHTML).toBe(''); + }); +}); From 508bc1be4f33c07ac1f50a573394caee3b790cc6 Mon Sep 17 00:00:00 2001 From: YONGJAE LEE Date: Sun, 30 Aug 2026 00:42:49 +0900 Subject: [PATCH 2/4] [ZEPPELIN-6650] Make the renderer specs fail on the routing they claim to pin --- .../components/renderers/HTMLRenderer.spec.tsx | 8 +++++--- .../templates/SingleResultRenderer.spec.tsx | 18 ++++++++++++++---- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/components/renderers/HTMLRenderer.spec.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/components/renderers/HTMLRenderer.spec.tsx index fd195971fe3..0911a9f15b9 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/components/renderers/HTMLRenderer.spec.tsx +++ b/zeppelin-web-angular/projects/zeppelin-react/src/components/renderers/HTMLRenderer.spec.tsx @@ -14,9 +14,11 @@ import { render, screen, waitFor } from '@testing-library/react'; import { describe, expect, it } from 'vitest'; import { HTMLRenderer } from './HTMLRenderer'; -// jsdom never runs scripts, so the execution this component exists for is out -// of reach here and belongs in e2e. These specs pin the swap that makes it -// possible: a script parsed out of innerHTML is inert, so it is rebuilt. +// jsdom never runs scripts, so the execution this component exists for is out of reach here and belongs in e2e. +// What these specs pin is the fidelity of the swap: +// attributes, body and position survive it. They do not prove the swap happened, +// because a script parsed out of innerHTML already carries all three. +// Only the async assertion below distinguishes a rebuilt script from an inert one. describe('HTMLRenderer', () => { it('renders the markup it is given', () => { render(); diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/templates/SingleResultRenderer.spec.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/templates/SingleResultRenderer.spec.tsx index dc4f60362c8..3240b42f11a 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/templates/SingleResultRenderer.spec.tsx +++ b/zeppelin-web-angular/projects/zeppelin-react/src/templates/SingleResultRenderer.spec.tsx @@ -26,10 +26,12 @@ const configs = { } as unknown as ParagraphConfigResults; describe('SingleResultRenderer', () => { - it('renders TEXT as preformatted output', () => { - render(); + it('renders TEXT as text, leaving markup in it literal', () => { + // Markup in the payload is what separates this arm from HTML: routing TEXT to + // HTMLRenderer would parse the tag away instead of showing it. + render(not bold')} />); - expect(screen.getByText(/line one/)).toBeTruthy(); + expect(screen.getByText(/line one not bold<\/b>/)).toBeTruthy(); }); it('renders TABLE through the visualization', () => { @@ -42,8 +44,16 @@ describe('SingleResultRenderer', () => { }); it('hands the visualization the display config for its own result index', () => { - render(); + // Both indices are rendered: index 1 alone would pass against a hard-coded [1], + // and the positive assertion keeps an absent chart from reading as success. + const table = render( + + ); + expect(screen.getByText('alice')).toBeTruthy(); + table.unmount(); + render(); + expect(screen.getByRole('button', { name: /Table/ })).toBeTruthy(); expect(screen.queryByText('alice')).toBeNull(); }); From dc3de466a4f89f8fc71cce2d2a6ff7e462170a83 Mon Sep 17 00:00:00 2001 From: YONGJAE LEE Date: Sun, 30 Aug 2026 00:42:49 +0900 Subject: [PATCH 3/4] [MINOR] Declare highlight.js in the React remote and handle a failed chunk --- .../projects/zeppelin-react/package-lock.json | 12 ++++++++++++ .../projects/zeppelin-react/package.json | 1 + .../src/components/renderers/HTMLRenderer.tsx | 9 ++++++--- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/zeppelin-web-angular/projects/zeppelin-react/package-lock.json b/zeppelin-web-angular/projects/zeppelin-react/package-lock.json index d68ffc83baa..2800b12b82a 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/package-lock.json +++ b/zeppelin-web-angular/projects/zeppelin-react/package-lock.json @@ -15,6 +15,7 @@ "chart.js": "^4.5.1", "date-fns": "^3.6.0", "file-saver": "2.0.5", + "highlight.js": "^9.15.8", "react": "18.3.1", "react-dom": "18.3.1", "rxjs": "^7.8.0", @@ -5519,6 +5520,17 @@ "he": "bin/he" } }, + "node_modules/highlight.js": { + "version": "9.18.5", + "resolved": "https://registry.npmjs.org/highlight.js/-/highlight.js-9.18.5.tgz", + "integrity": "sha512-a5bFyofd/BHCX52/8i8uJkjr9DYwXIPnM/plwI6W7ezItLGqzt7X2G2nXuYSfsIJdkwwj/g9DG1LkcGJI/dDoA==", + "deprecated": "Support has ended for 9.x series. Upgrade to @latest", + "hasInstallScript": true, + "license": "BSD-3-Clause", + "engines": { + "node": "*" + } + }, "node_modules/html-encoding-sniffer": { "version": "6.0.0", "resolved": "https://registry.npmjs.org/html-encoding-sniffer/-/html-encoding-sniffer-6.0.0.tgz", diff --git a/zeppelin-web-angular/projects/zeppelin-react/package.json b/zeppelin-web-angular/projects/zeppelin-react/package.json index 1d99fda2530..df02dab7c6e 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/package.json +++ b/zeppelin-web-angular/projects/zeppelin-react/package.json @@ -17,6 +17,7 @@ "@ant-design/icons": "5.4.0", "@zeppelin/sdk": "file:../zeppelin-sdk", "ansi-to-react": "6.2.6", + "highlight.js": "^9.15.8", "antd": "5.21.0", "chart.js": "^4.5.1", "date-fns": "^3.6.0", diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/components/renderers/HTMLRenderer.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/components/renderers/HTMLRenderer.tsx index 45fbeec9d40..93ba5b52230 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/components/renderers/HTMLRenderer.tsx +++ b/zeppelin-web-angular/projects/zeppelin-react/src/components/renderers/HTMLRenderer.tsx @@ -31,9 +31,12 @@ export const HTMLRenderer = ({ html }: HTMLRendererProps) => { // Highlight code blocks (matches Angular: result.component.ts renderHTML) const codeEle = container.querySelector('pre code'); if (codeEle) { - import('highlight.js').then(({ default: hljs }) => { - hljs.highlightBlock(codeEle as HTMLElement); - }); + import('highlight.js') + .then(({ default: hljs }) => { + hljs.highlightBlock(codeEle as HTMLElement); + }) + // Without this a failed chunk is an unhandled rejection; the cost is an unhighlighted block. + .catch(() => undefined); } const scripts = Array.from(container.querySelectorAll('script')); From 10ebf7d2a8b2fffb8ce094cb4d485489f9fab3f8 Mon Sep 17 00:00:00 2001 From: kimyenac Date: Sun, 30 Aug 2026 14:32:38 +0900 Subject: [PATCH 4/4] [MINOR] Cover the failed highlight chunk The catch added alongside the highlight.js declaration had nothing pointing at it. Mocking the module to throw makes the dynamic import reject, which leaves the code block unhighlighted and the render otherwise intact. It needs a file of its own because vi.mock is hoisted over the whole module, so it cannot sit beside the specs that need highlighting to work. Removing the catch again turns the rejection loose and the run exits 1, which is the regression this guards. --- .../HTMLRenderer.failedHighlight.spec.tsx | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 zeppelin-web-angular/projects/zeppelin-react/src/components/renderers/HTMLRenderer.failedHighlight.spec.tsx diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/components/renderers/HTMLRenderer.failedHighlight.spec.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/components/renderers/HTMLRenderer.failedHighlight.spec.tsx new file mode 100644 index 00000000000..77984620d62 --- /dev/null +++ b/zeppelin-web-angular/projects/zeppelin-react/src/components/renderers/HTMLRenderer.failedHighlight.spec.tsx @@ -0,0 +1,33 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { render } from '@testing-library/react'; +import { describe, expect, it, vi } from 'vitest'; +import { HTMLRenderer } from './HTMLRenderer'; + +// A file of its own: the mock is hoisted over the whole module, so it cannot +// share one with the specs that need highlighting to work. +vi.mock('highlight.js', () => { + throw new Error('chunk load failed'); +}); + +describe('HTMLRenderer when the highlight chunk fails', () => { + it('leaves the code block unhighlighted instead of raising an unhandled rejection', async () => { + const { container } = render(); + + // Give the rejected import a turn to settle; nothing must escape it. + await vi.waitFor(() => expect(container.querySelector('pre code')).not.toBeNull()); + + expect(container.querySelector('pre code')!.classList.contains('hljs')).toBe(false); + expect(container.textContent).toContain('const x = 1;'); + }); +});