From cc14bc4515279b9857bc10f08d485035b6e3bad6 Mon Sep 17 00:00:00 2001 From: tisilent Date: Wed, 13 Sep 2023 17:34:31 +0800 Subject: [PATCH 1/8] using isCursorInitialized for domrenderer --- src/browser/renderer/dom/DomRendererRowFactory.ts | 2 +- test/playwright/SharedRendererTests.ts | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/browser/renderer/dom/DomRendererRowFactory.ts b/src/browser/renderer/dom/DomRendererRowFactory.ts index 614b2301..6ab68e7d 100644 --- a/src/browser/renderer/dom/DomRendererRowFactory.ts +++ b/src/browser/renderer/dom/DomRendererRowFactory.ts @@ -218,7 +218,7 @@ export class DomRendererRowFactory { } } - if (!this._coreService.isCursorHidden && isCursorCell) { + if (!this._coreService.isCursorHidden && isCursorCell && this._coreService.isCursorInitialized) { classes.push(RowCss.CURSOR_CLASS); if (this._coreBrowserService.isFocused) { if (cursorBlink) { diff --git a/test/playwright/SharedRendererTests.ts b/test/playwright/SharedRendererTests.ts index e1e56bc9..1e7648bb 100644 --- a/test/playwright/SharedRendererTests.ts +++ b/test/playwright/SharedRendererTests.ts @@ -1147,6 +1147,17 @@ export function injectSharedRendererTests(ctx: ISharedRendererTestContext): void await ctx.value.proxy.selectAll(); await pollFor(ctx.value.page, () => getCellColor(ctx.value, 1, 1), [0, 0, 255, 255]); }); + test('#4790: cursor should not be displayed before focusing', async () => { + const theme: ITheme = { + cursor: '#0000FF' + }; + await ctx.value.page.evaluate(`window.term.options.theme = ${JSON.stringify(theme)};`); + await pollFor(ctx.value.page, () => getCellColor(ctx.value, 1, 1), [0, 0, 0, 0]); + await ctx.value.proxy.focus(); + await pollFor(ctx.value.page, () => getCellColor(ctx.value, 1, 1), [0, 0, 255, 255]); + await ctx.value.proxy.blur(); + await pollFor(ctx.value.page, () => getCellColor(ctx.value, 1, 1), [0, 0, 255, 255]); + }); }); } From 457060f89dcf7416de73a428730d3f4449ae7512 Mon Sep 17 00:00:00 2001 From: tisilent Date: Wed, 13 Sep 2023 18:15:37 +0800 Subject: [PATCH 2/8] Modify the isCursorInitialized default true --- src/common/TestUtils.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/TestUtils.test.ts b/src/common/TestUtils.test.ts index fcf0c2cd..8e57664e 100644 --- a/src/common/TestUtils.test.ts +++ b/src/common/TestUtils.test.ts @@ -78,7 +78,7 @@ export class MockCharsetService implements ICharsetService { export class MockCoreService implements ICoreService { public serviceBrand: any; - public isCursorInitialized: boolean = false; + public isCursorInitialized: boolean = true; public isCursorHidden: boolean = false; public isFocused: boolean = false; public modes: IModes = { From a1229c78a21e3aea24c79fc9fb0a7651a02c746b Mon Sep 17 00:00:00 2001 From: tisilent Date: Wed, 13 Sep 2023 18:43:04 +0800 Subject: [PATCH 3/8] Add cursor initialize test --- .../renderer/dom/DomRendererRowFactory.test.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/browser/renderer/dom/DomRendererRowFactory.test.ts b/src/browser/renderer/dom/DomRendererRowFactory.test.ts index 36923dbb..dfea8b01 100644 --- a/src/browser/renderer/dom/DomRendererRowFactory.test.ts +++ b/src/browser/renderer/dom/DomRendererRowFactory.test.ts @@ -96,6 +96,24 @@ describe('DomRendererRowFactory', () => { } }); + it('should not display cursor for before initializing', () => { + const coreService = new MockCoreService(); + coreService.isCursorInitialized = false; + const rowFactory = new DomRendererRowFactory( + dom.window.document, + new MockCharacterJoinerService(), + new MockOptionsService(), + new MockCoreBrowserService(), + coreService, + new MockDecorationService(), + new MockThemeService() + ); + const spans = rowFactory.createRow(lineData, 0, true, 'block', undefined, 0, false, 5, EMPTY_WIDTH, -1, -1); + assert.equal(extractHtml(spans), + ` ` + ); + }); + describe('attributes', () => { it('should add class for bold', () => { const cell = CellData.fromCharData([0, 'a', 1, 'a'.charCodeAt(0)]); From 871400f4075d331bc6fa5e03ff07d1c58a03668f Mon Sep 17 00:00:00 2001 From: tisilent Date: Wed, 13 Sep 2023 23:28:37 +0800 Subject: [PATCH 4/8] Open a new page to perform testing. --- test/playwright/SharedRendererTests.ts | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/test/playwright/SharedRendererTests.ts b/test/playwright/SharedRendererTests.ts index 1e7648bb..ae35fc28 100644 --- a/test/playwright/SharedRendererTests.ts +++ b/test/playwright/SharedRendererTests.ts @@ -6,7 +6,7 @@ import { IImage32, decodePng } from '@lunapaint/png-codec'; import { LocatorScreenshotOptions, test } from '@playwright/test'; import { ITheme } from 'xterm'; -import { ITestContext, MaybeAsync, pollFor, pollForApproximate } from './TestUtils'; +import { ITestContext, MaybeAsync, createTestContext, openTerminal, pollFor, pollForApproximate } from './TestUtils'; export interface ISharedRendererTestContext { value: ITestContext; @@ -1151,12 +1151,18 @@ export function injectSharedRendererTests(ctx: ISharedRendererTestContext): void const theme: ITheme = { cursor: '#0000FF' }; - await ctx.value.page.evaluate(`window.term.options.theme = ${JSON.stringify(theme)};`); - await pollFor(ctx.value.page, () => getCellColor(ctx.value, 1, 1), [0, 0, 0, 0]); - await ctx.value.proxy.focus(); - await pollFor(ctx.value.page, () => getCellColor(ctx.value, 1, 1), [0, 0, 255, 255]); - await ctx.value.proxy.blur(); - await pollFor(ctx.value.page, () => getCellColor(ctx.value, 1, 1), [0, 0, 255, 255]); + const octx = await createTestContext(ctx.value.browser); + await openTerminal(octx); + await octx.page.evaluate(`window.term.options.theme = ${JSON.stringify(theme)};`); + frameDetails = undefined; + await pollFor(octx.page, () => getCellColor(octx, 1, 1), [0, 0, 0, 255]); + await octx.proxy.focus(); + frameDetails = undefined; + await pollFor(octx.page, () => getCellColor(octx, 1, 1), [0, 0, 255, 255]); + await octx.proxy.blur(); + frameDetails = undefined; + await pollFor(octx.page, () => getCellColor(octx, 1, 1), [0, 0, 0, 255]); + octx.page.close(); }); }); } From 27d89c231127e89ff4271ed38f1e763d8da8ef1b Mon Sep 17 00:00:00 2001 From: tisilent Date: Thu, 14 Sep 2023 00:47:37 +0800 Subject: [PATCH 5/8] Reduce side effects --- test/playwright/Renderer.test.ts | 4 +-- test/playwright/SharedRendererTests.ts | 36 +++++++++++++++++++------- 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/test/playwright/Renderer.test.ts b/test/playwright/Renderer.test.ts index c5c1e94d..965b1640 100644 --- a/test/playwright/Renderer.test.ts +++ b/test/playwright/Renderer.test.ts @@ -5,7 +5,7 @@ import { test } from '@playwright/test'; import { ITestContext, createTestContext, openTerminal } from './TestUtils'; -import { ISharedRendererTestContext, injectSharedRendererTests } from './SharedRendererTests'; +import { ISharedRendererTestContext, injectSharedRendererOnceTests, injectSharedRendererTests } from './SharedRendererTests'; let ctx: ITestContext; const ctxWrapper: ISharedRendererTestContext = { value: undefined } as any; @@ -18,5 +18,5 @@ test.afterAll(async () => await ctx.page.close()); test.describe('DOM Renderer Integration Tests', () => { injectSharedRendererTests(ctxWrapper); + injectSharedRendererOnceTests(ctxWrapper); }); - diff --git a/test/playwright/SharedRendererTests.ts b/test/playwright/SharedRendererTests.ts index ae35fc28..261ad6e8 100644 --- a/test/playwright/SharedRendererTests.ts +++ b/test/playwright/SharedRendererTests.ts @@ -1147,22 +1147,38 @@ export function injectSharedRendererTests(ctx: ISharedRendererTestContext): void await ctx.value.proxy.selectAll(); await pollFor(ctx.value.page, () => getCellColor(ctx.value, 1, 1), [0, 0, 255, 255]); }); + }); +} + +export function injectSharedRendererOnceTests(inctx: ISharedRendererTestContext): void { + let ctx: ITestContext; + test.beforeEach(async () => { + ctx = await createTestContext(inctx.value.browser); + await openTerminal(ctx); + ctx.page.evaluate(` + window.term.options.minimumContrastRatio = 1; + window.term.options.allowTransparency = false; + window.term.options.theme = undefined; + `); + // Clear the cached screenshot before each test + frameDetails = undefined; + }); + test.afterEach(async () => { + ctx.page.close(); + }); + test.describe('regression tests', () => { test('#4790: cursor should not be displayed before focusing', async () => { const theme: ITheme = { cursor: '#0000FF' }; - const octx = await createTestContext(ctx.value.browser); - await openTerminal(octx); - await octx.page.evaluate(`window.term.options.theme = ${JSON.stringify(theme)};`); + await ctx.page.evaluate(`window.term.options.theme = ${JSON.stringify(theme)};`); + await pollFor(ctx.page, () => getCellColor(ctx, 1, 1), [0, 0, 0, 255]); + await ctx.proxy.focus(); frameDetails = undefined; - await pollFor(octx.page, () => getCellColor(octx, 1, 1), [0, 0, 0, 255]); - await octx.proxy.focus(); + await pollFor(ctx.page, () => getCellColor(ctx, 1, 1), [0, 0, 255, 255]); + await ctx.proxy.blur(); frameDetails = undefined; - await pollFor(octx.page, () => getCellColor(octx, 1, 1), [0, 0, 255, 255]); - await octx.proxy.blur(); - frameDetails = undefined; - await pollFor(octx.page, () => getCellColor(octx, 1, 1), [0, 0, 0, 255]); - octx.page.close(); + await pollFor(ctx.page, () => getCellColor(ctx, 1, 1), [0, 0, 0, 255]); }); }); } From 49dc046b9f246dd972cdc3f125a5c17119899760 Mon Sep 17 00:00:00 2001 From: tisilent Date: Thu, 14 Sep 2023 09:52:58 +0800 Subject: [PATCH 6/8] rename and remove new page --- test/playwright/Renderer.test.ts | 4 ++-- test/playwright/SharedRendererTests.ts | 26 +++++++++++--------------- 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/test/playwright/Renderer.test.ts b/test/playwright/Renderer.test.ts index 965b1640..77bf7941 100644 --- a/test/playwright/Renderer.test.ts +++ b/test/playwright/Renderer.test.ts @@ -5,7 +5,7 @@ import { test } from '@playwright/test'; import { ITestContext, createTestContext, openTerminal } from './TestUtils'; -import { ISharedRendererTestContext, injectSharedRendererOnceTests, injectSharedRendererTests } from './SharedRendererTests'; +import { ISharedRendererTestContext, injectSharedRendererTestsStandalone, injectSharedRendererTests } from './SharedRendererTests'; let ctx: ITestContext; const ctxWrapper: ISharedRendererTestContext = { value: undefined } as any; @@ -18,5 +18,5 @@ test.afterAll(async () => await ctx.page.close()); test.describe('DOM Renderer Integration Tests', () => { injectSharedRendererTests(ctxWrapper); - injectSharedRendererOnceTests(ctxWrapper); + injectSharedRendererTestsStandalone(ctxWrapper); }); diff --git a/test/playwright/SharedRendererTests.ts b/test/playwright/SharedRendererTests.ts index 261ad6e8..b29f45cf 100644 --- a/test/playwright/SharedRendererTests.ts +++ b/test/playwright/SharedRendererTests.ts @@ -6,7 +6,7 @@ import { IImage32, decodePng } from '@lunapaint/png-codec'; import { LocatorScreenshotOptions, test } from '@playwright/test'; import { ITheme } from 'xterm'; -import { ITestContext, MaybeAsync, createTestContext, openTerminal, pollFor, pollForApproximate } from './TestUtils'; +import { ITestContext, MaybeAsync, openTerminal, pollFor, pollForApproximate } from './TestUtils'; export interface ISharedRendererTestContext { value: ITestContext; @@ -1150,12 +1150,11 @@ export function injectSharedRendererTests(ctx: ISharedRendererTestContext): void }); } -export function injectSharedRendererOnceTests(inctx: ISharedRendererTestContext): void { - let ctx: ITestContext; +export function injectSharedRendererTestsStandalone(ctx: ISharedRendererTestContext): void { test.beforeEach(async () => { - ctx = await createTestContext(inctx.value.browser); - await openTerminal(ctx); - ctx.page.evaluate(` + // Recreate terminal + await openTerminal(ctx.value); + ctx.value.page.evaluate(` window.term.options.minimumContrastRatio = 1; window.term.options.allowTransparency = false; window.term.options.theme = undefined; @@ -1163,22 +1162,19 @@ export function injectSharedRendererOnceTests(inctx: ISharedRendererTestContext) // Clear the cached screenshot before each test frameDetails = undefined; }); - test.afterEach(async () => { - ctx.page.close(); - }); test.describe('regression tests', () => { test('#4790: cursor should not be displayed before focusing', async () => { const theme: ITheme = { cursor: '#0000FF' }; - await ctx.page.evaluate(`window.term.options.theme = ${JSON.stringify(theme)};`); - await pollFor(ctx.page, () => getCellColor(ctx, 1, 1), [0, 0, 0, 255]); - await ctx.proxy.focus(); + await ctx.value.page.evaluate(`window.term.options.theme = ${JSON.stringify(theme)};`); + await pollFor(ctx.value.page, () => getCellColor(ctx.value, 1, 1), [0, 0, 0, 255]); + await ctx.value.proxy.focus(); frameDetails = undefined; - await pollFor(ctx.page, () => getCellColor(ctx, 1, 1), [0, 0, 255, 255]); - await ctx.proxy.blur(); + await pollFor(ctx.value.page, () => getCellColor(ctx.value, 1, 1), [0, 0, 255, 255]); + await ctx.value.proxy.blur(); frameDetails = undefined; - await pollFor(ctx.page, () => getCellColor(ctx, 1, 1), [0, 0, 0, 255]); + await pollFor(ctx.value.page, () => getCellColor(ctx.value, 1, 1), [0, 0, 0, 255]); }); }); } From 66c8e33b478d34776b2409367a185a4a160f1749 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Thu, 14 Sep 2023 12:33:34 -0700 Subject: [PATCH 7/8] Add standalone renderer tests to canvas/webgl --- addons/xterm-addon-canvas/test/CanvasRenderer.test.ts | 3 ++- addons/xterm-addon-webgl/test/WebglRenderer.test.ts | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/addons/xterm-addon-canvas/test/CanvasRenderer.test.ts b/addons/xterm-addon-canvas/test/CanvasRenderer.test.ts index 76f57394..2782081c 100644 --- a/addons/xterm-addon-canvas/test/CanvasRenderer.test.ts +++ b/addons/xterm-addon-canvas/test/CanvasRenderer.test.ts @@ -4,7 +4,7 @@ */ import test from '@playwright/test'; -import { ISharedRendererTestContext, injectSharedRendererTests } from '../../../out-test/playwright/SharedRendererTests'; +import { ISharedRendererTestContext, injectSharedRendererTests, injectSharedRendererTestsStandalone } from '../../../out-test/playwright/SharedRendererTests'; import { ITestContext, createTestContext, openTerminal } from '../../../out-test/playwright/TestUtils'; let ctx: ITestContext; @@ -28,4 +28,5 @@ test.describe('Canvas Renderer Integration Tests', () => { test.skip(({ browserName }) => browserName === 'webkit'); injectSharedRendererTests(ctxWrapper); + injectSharedRendererTestsStandalone(ctxWrapper); }); diff --git a/addons/xterm-addon-webgl/test/WebglRenderer.test.ts b/addons/xterm-addon-webgl/test/WebglRenderer.test.ts index 718d8e03..4b73c08b 100644 --- a/addons/xterm-addon-webgl/test/WebglRenderer.test.ts +++ b/addons/xterm-addon-webgl/test/WebglRenderer.test.ts @@ -4,7 +4,7 @@ */ import test from '@playwright/test'; -import { ISharedRendererTestContext, injectSharedRendererTests } from '../../../out-test/playwright/SharedRendererTests'; +import { ISharedRendererTestContext, injectSharedRendererTests, injectSharedRendererTestsStandalone } from '../../../out-test/playwright/SharedRendererTests'; import { ITestContext, createTestContext, openTerminal } from '../../../out-test/playwright/TestUtils'; import { platform } from 'os'; @@ -29,4 +29,5 @@ test.describe('WebGL Renderer Integration Tests', async () => { } injectSharedRendererTests(ctxWrapper); + injectSharedRendererTestsStandalone(ctxWrapper); }); From 2f9439b6907b1796f74693d804da657082ea215f Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Thu, 14 Sep 2023 12:35:12 -0700 Subject: [PATCH 8/8] Explain injectSharedRendererTestsStandalone --- test/playwright/SharedRendererTests.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/playwright/SharedRendererTests.ts b/test/playwright/SharedRendererTests.ts index b29f45cf..76c5051b 100644 --- a/test/playwright/SharedRendererTests.ts +++ b/test/playwright/SharedRendererTests.ts @@ -1150,6 +1150,11 @@ export function injectSharedRendererTests(ctx: ISharedRendererTestContext): void }); } +/** + * Injects shared renderer tests where it's required to re-initialize the terminal for each test. + * This is much slower than just calling `Terminal.reset` but testing some features needs this + * treatment. + */ export function injectSharedRendererTestsStandalone(ctx: ISharedRendererTestContext): void { test.beforeEach(async () => { // Recreate terminal