From 471b4bafc4c0f3bb4745de8d47c10e304515d41b Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 18 May 2026 16:48:31 -0500 Subject: [PATCH] fix(tests): resolve jest 30.4 + jsdom 26 compatibility issues - Patch Object.defineProperties in jsDomWithFetchAPI.ts to make window.location configurable, enabling jest.spyOn in tests with jest-environment-jsdom 30 (which bundles jsdom 26) - Update TableChart.test.tsx: jsdom 26 returns rgba(0,0,0,0) for elements with no background, replacing empty string expectations - Apply prettier formatting fixes to logger.test.ts and RedirectWarning/utils.test.ts Co-Authored-By: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-Authored-By: Claude Sonnet 4.6 --- .../test/TableChart.test.tsx | 56 +++++++++++++------ .../spec/helpers/jsDomWithFetchAPI.ts | 32 +++++++++++ .../src/middleware/logger.test.ts | 10 ++-- .../src/pages/RedirectWarning/utils.test.ts | 6 +- 4 files changed, 79 insertions(+), 25 deletions(-) diff --git a/superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx b/superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx index 1202e7a8fb6..474298599fd 100644 --- a/superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx +++ b/superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx @@ -632,7 +632,9 @@ describe('plugin-chart-table', () => { expect(getComputedStyle(screen.getByTitle('2467063')).background).toBe( 'rgb(172, 225, 196)', ); - expect(getComputedStyle(screen.getByTitle('2467')).background).toBe(''); + expect(getComputedStyle(screen.getByTitle('2467')).background).toBe( + 'rgba(0, 0, 0, 0)', + ); }); test('render cell without color', () => { @@ -672,9 +674,11 @@ describe('plugin-chart-table', () => { 'rgba(172, 225, 196, 0.81)', ); expect(getComputedStyle(screen.getByTitle('2467063')).background).toBe( - '', + 'rgba(0, 0, 0, 0)', + ); + expect(getComputedStyle(screen.getByText('N/A')).background).toBe( + 'rgba(0, 0, 0, 0)', ); - expect(getComputedStyle(screen.getByText('N/A')).background).toBe(''); }); test('preserves muted null styling when no formatter resolves text color', () => { @@ -1062,7 +1066,7 @@ describe('plugin-chart-table', () => { 'rgb(172, 225, 196)', ); expect(getComputedStyle(screen.getByText('Michael')).background).toBe( - '', + 'rgba(0, 0, 0, 0)', ); }); @@ -1092,7 +1096,9 @@ describe('plugin-chart-table', () => { expect(getComputedStyle(screen.getByText('Maria')).background).toBe( 'rgb(172, 225, 196)', ); - expect(getComputedStyle(screen.getByText('Joe')).background).toBe(''); + expect(getComputedStyle(screen.getByText('Joe')).background).toBe( + 'rgba(0, 0, 0, 0)', + ); }); test('render color with string column color formatter (operator containing)', () => { @@ -1121,7 +1127,9 @@ describe('plugin-chart-table', () => { expect(getComputedStyle(screen.getByText('Michael')).background).toBe( 'rgb(172, 225, 196)', ); - expect(getComputedStyle(screen.getByText('Joe')).background).toBe(''); + expect(getComputedStyle(screen.getByText('Joe')).background).toBe( + 'rgba(0, 0, 0, 0)', + ); }); test('render color with string column color formatter (operator not containing)', () => { @@ -1151,7 +1159,7 @@ describe('plugin-chart-table', () => { 'rgb(172, 225, 196)', ); expect(getComputedStyle(screen.getByText('Michael')).background).toBe( - '', + 'rgba(0, 0, 0, 0)', ); }); @@ -1182,7 +1190,7 @@ describe('plugin-chart-table', () => { 'rgb(172, 225, 196)', ); expect(getComputedStyle(screen.getByText('Michael')).background).toBe( - '', + 'rgba(0, 0, 0, 0)', ); }); @@ -1245,7 +1253,9 @@ describe('plugin-chart-table', () => { expect(getComputedStyle(screen.getByText('true')).background).toBe( 'rgb(172, 225, 196)', ); - expect(getComputedStyle(screen.getByText('false')).background).toBe(''); + expect(getComputedStyle(screen.getByText('false')).background).toBe( + 'rgba(0, 0, 0, 0)', + ); }); test('render color with boolean column color formatter (operator is false)', () => { @@ -1274,7 +1284,9 @@ describe('plugin-chart-table', () => { expect(getComputedStyle(screen.getByText('false')).background).toBe( 'rgb(172, 225, 196)', ); - expect(getComputedStyle(screen.getByText('true')).background).toBe(''); + expect(getComputedStyle(screen.getByText('true')).background).toBe( + 'rgba(0, 0, 0, 0)', + ); }); test('render color with boolean column color formatter (operator is null)', () => { @@ -1303,8 +1315,12 @@ describe('plugin-chart-table', () => { expect(getComputedStyle(screen.getByText('N/A')).background).toBe( 'rgb(172, 225, 196)', ); - expect(getComputedStyle(screen.getByText('true')).background).toBe(''); - expect(getComputedStyle(screen.getByText('false')).background).toBe(''); + expect(getComputedStyle(screen.getByText('true')).background).toBe( + 'rgba(0, 0, 0, 0)', + ); + expect(getComputedStyle(screen.getByText('false')).background).toBe( + 'rgba(0, 0, 0, 0)', + ); }); test('render color with boolean column color formatter (operator is not null)', () => { @@ -1338,7 +1354,9 @@ describe('plugin-chart-table', () => { expect(getComputedStyle(falseElements[0]).background).toBe( 'rgb(172, 225, 196)', ); - expect(getComputedStyle(screen.getByText('N/A')).background).toBe(''); + expect(getComputedStyle(screen.getByText('N/A')).background).toBe( + 'rgba(0, 0, 0, 0)', + ); }); test('render color with column color formatter to entire row', () => { @@ -1644,7 +1662,9 @@ describe('plugin-chart-table', () => { expect(getComputedStyle(screen.getByTitle('2467063')).background).toBe( 'rgb(172, 225, 196)', ); - expect(getComputedStyle(screen.getByTitle('2467')).background).toBe(''); + expect(getComputedStyle(screen.getByTitle('2467')).background).toBe( + 'rgba(0, 0, 0, 0)', + ); }); test('render color with useGradient true returns gradient color', () => { @@ -1676,7 +1696,9 @@ describe('plugin-chart-table', () => { expect(getComputedStyle(screen.getByTitle('2467063')).background).toBe( 'rgb(172, 225, 196)', ); - expect(getComputedStyle(screen.getByTitle('2467')).background).toBe(''); + expect(getComputedStyle(screen.getByTitle('2467')).background).toBe( + 'rgba(0, 0, 0, 0)', + ); }); test('render color with useGradient undefined defaults to gradient (backward compatibility)', () => { @@ -1707,7 +1729,9 @@ describe('plugin-chart-table', () => { expect(getComputedStyle(screen.getByTitle('2467063')).background).toBe( 'rgb(172, 225, 196)', ); - expect(getComputedStyle(screen.getByTitle('2467')).background).toBe(''); + expect(getComputedStyle(screen.getByTitle('2467')).background).toBe( + 'rgba(0, 0, 0, 0)', + ); }); test('render color with useGradient false and None operator returns solid color', () => { diff --git a/superset-frontend/spec/helpers/jsDomWithFetchAPI.ts b/superset-frontend/spec/helpers/jsDomWithFetchAPI.ts index 1db1bafac38..5c67e5080e2 100644 --- a/superset-frontend/spec/helpers/jsDomWithFetchAPI.ts +++ b/superset-frontend/spec/helpers/jsDomWithFetchAPI.ts @@ -19,6 +19,38 @@ import JSDOMEnvironment from 'jest-environment-jsdom'; +// jest-environment-jsdom 30 bundles jsdom 26, which marks window.location as +// [LegacyUnforgeable] (configurable: false). jest 30's spyOn now strictly +// checks the configurable flag and throws when it's false, breaking every test +// that uses jest.spyOn(window, 'location', 'get'). +// +// We intercept Object.defineProperties at module-load time (before any JSDOM +// instance is created). The interceptor makes window.location configurable +// every time jsdom creates a new Window, restoring the ability to spy on it. +// This file is only required by Jest in the test environment so the +// monkey-patch is safe. +const _originalDefineProperties = Object.defineProperties.bind(Object); +(Object as any).defineProperties = function ( + obj: object, + props: PropertyDescriptorMap, +) { + if ( + props !== null && + typeof props === 'object' && + Object.prototype.hasOwnProperty.call(props, 'location') && + (props as any).location?.configurable === false + ) { + // Allow jest.spyOn(window, 'location', 'get') to work in tests by making + // the property configurable. This deviates from the browser spec's + // [LegacyUnforgeable] requirement but is acceptable in a test environment. + props = { + ...props, + location: { ...(props as any).location, configurable: true }, + }; + } + return _originalDefineProperties(obj, props); +}; + // https://github.com/facebook/jest/blob/v29.4.3/website/versioned_docs/version-29.4/Configuration.md#testenvironment-string export default class FixJSDOMEnvironment extends JSDOMEnvironment { constructor(...args: ConstructorParameters) { diff --git a/superset-frontend/src/middleware/logger.test.ts b/superset-frontend/src/middleware/logger.test.ts index b1296a5c451..f54c154d54d 100644 --- a/superset-frontend/src/middleware/logger.test.ts +++ b/superset-frontend/src/middleware/logger.test.ts @@ -99,12 +99,10 @@ describe('logger middleware', () => { test('should include ts, start_offset, event_name, impression_id, source, and source_id in every event', () => { // Set window.location to include /dashboard/ so the middleware adds dashboard context - const locationSpy = jest - .spyOn(window, 'location', 'get') - .mockReturnValue({ - ...window.location, - href: `http://localhost/dashboard/${dashboardId}/`, - } as Location); + const locationSpy = jest.spyOn(window, 'location', 'get').mockReturnValue({ + ...window.location, + href: `http://localhost/dashboard/${dashboardId}/`, + } as Location); try { const fetchLog = (logger as Function)(mockStore)(next); diff --git a/superset-frontend/src/pages/RedirectWarning/utils.test.ts b/superset-frontend/src/pages/RedirectWarning/utils.test.ts index beea431964b..0cb38a2d80f 100644 --- a/superset-frontend/src/pages/RedirectWarning/utils.test.ts +++ b/superset-frontend/src/pages/RedirectWarning/utils.test.ts @@ -57,9 +57,9 @@ test('isAllowedScheme allows relative URLs (unparseable as absolute)', () => { }); test('getTargetUrl reads the url query parameter', () => { - const locationSpy = jest - .spyOn(window, 'location', 'get') - .mockReturnValue({ search: '?url=https%3A%2F%2Fexample.com%2Fpage' } as Location); + const locationSpy = jest.spyOn(window, 'location', 'get').mockReturnValue({ + search: '?url=https%3A%2F%2Fexample.com%2Fpage', + } as Location); expect(getTargetUrl()).toBe('https://example.com/page'); locationSpy.mockRestore(); });