Skip to content

Commit

Permalink
Enqueue notebook renderer webview renders immediately (#4475)
Browse files Browse the repository at this point in the history
This is another attempt to fix the failing smoke tests for IPyWidget
plots (#4448).

We probably should have always been calling `enqueue` instead of
`enqueueIdle`, since we only render a single output item in our notebook
output webviews.

I also enabled renderer logging for debugging issues like this in
future.

### QA Notes

Plot tests in the full test suite passed 6 times on this branch:
https://github.com/posit-dev/positron/actions/runs/10529737503.
  • Loading branch information
seeM authored Aug 26, 2024
1 parent 9faaed1 commit ac7fdd2
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1638,7 +1638,7 @@ async function webviewPreloads(ctx: PreloadContext) {
// --- Start Positron ---
case 'positronRender': {
const data = event.data;
outputRunner.enqueueIdle(data.outputId, async signal => {
outputRunner.enqueue(data.outputId, async signal => {
// Get the element to render into.
const element = document.getElementById(data.elementId);
if (!element) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { FromWebviewMessage, IClickedDataUrlMessage } from 'vs/workbench/contrib
import { IScopedRendererMessaging } from 'vs/workbench/contrib/notebook/common/notebookRendererMessagingService';
import { INotebookOutputWebview } from 'vs/workbench/contrib/positronOutputWebview/browser/notebookOutputWebviewService';
import { IOverlayWebview, IWebviewElement } from 'vs/workbench/contrib/webview/browser/webview';
import { INotebookLoggingService } from 'vs/workbench/contrib/notebook/common/notebookLoggingService';

interface NotebookOutputWebviewOptions<WType extends IOverlayWebview | IWebviewElement = IOverlayWebview> {
readonly id: string;
Expand Down Expand Up @@ -70,6 +71,7 @@ export class NotebookOutputWebview<WType extends IOverlayWebview | IWebviewEleme
@IFileService private _fileService: IFileService,
@IWorkspaceContextService private _workspaceContextService: IWorkspaceContextService,
@ILogService private _logService: ILogService,
@INotebookLoggingService private _notebookLogService: INotebookLoggingService,
@INotificationService private _notificationService: INotificationService,
) {
super();
Expand Down Expand Up @@ -109,6 +111,14 @@ export class NotebookOutputWebview<WType extends IOverlayWebview | IWebviewEleme
case 'customRendererMessage':
rendererMessaging?.postMessage(data.rendererId, data.message);
break;
case 'logRendererDebugMessage':
this._notebookLogService.debug(
'NotebookOutputWebview',
`${this.sessionId} (${this.id}) - ` +
data.message +
data.data ? ' ' + JSON.stringify(data.data, null, 4) : ''
);
break;
case 'positronRenderComplete':
this._onDidRender.fire();
break;
Expand Down
12 changes: 4 additions & 8 deletions test/smoke/src/areas/positron/plots/plots.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -304,8 +304,7 @@ plt.show()`;
await app.workbench.positronPlots.waitForNoPlots();
});

// skipping for now as we have intermittent issues with blank plots appearing
it.skip('Python - Verifies bqplot Python widget [C720869]', async function () {
it('Python - Verifies bqplot Python widget [C720869]', async function () {
const app = this.app as Application;

const script = `import bqplot.pyplot as bplt
Expand All @@ -326,8 +325,7 @@ bplt.show()`;

});

// skipping for now as we have intermittent issues with blank plots appearing
it.skip('Python - Verifies ipydatagrid Python widget [C720870]', async function () {
it('Python - Verifies ipydatagrid Python widget [C720870]', async function () {
const app = this.app as Application;

const script = `import pandas as pd
Expand All @@ -340,8 +338,7 @@ DataGrid(data, selection_mode="cell", editable=True)`;

});

// skipping for now as we have intermittent issues with blank plots appearing
it.skip('Python - Verifies ipyleaflet Python widget [C720871]', async function () {
it('Python - Verifies ipyleaflet Python widget [C720871]', async function () {
const app = this.app as Application;

const script = `from ipyleaflet import Map, Marker, display
Expand All @@ -359,8 +356,7 @@ display(map)`;

});

// skipping for now as we have intermittent issues with blank plots appearing
it.skip('Python - Verifies ipytree Python widget [C720872]', async function () {
it('Python - Verifies ipytree Python widget [C720872]', async function () {
const app = this.app as Application;

const script = `from ipytree import Tree, Node
Expand Down

0 comments on commit ac7fdd2

Please sign in to comment.