From fcd51fe155fc0062c6b205c9f05df3ea039c0177 Mon Sep 17 00:00:00 2001 From: Sean Perkins Date: Sat, 23 Mar 2024 18:49:30 -0400 Subject: [PATCH 1/5] chore(playwright): combine test configs for themes and modes --- core/src/utils/test/playwright/generator.ts | 73 +++++++++++++-------- 1 file changed, 47 insertions(+), 26 deletions(-) diff --git a/core/src/utils/test/playwright/generator.ts b/core/src/utils/test/playwright/generator.ts index 0c54407890d..b1da8a8b59a 100644 --- a/core/src/utils/test/playwright/generator.ts +++ b/core/src/utils/test/playwright/generator.ts @@ -1,6 +1,4 @@ -import { isModeValidForTheme } from '../../../global/ionic-global'; - -export type Mode = 'ios' | 'md'; +export type Mode = 'ios' | 'md' | 'ionic-ios' | 'ionic-md'; export type Direction = 'ltr' | 'rtl'; export type Theme = 'ios' | 'md' | 'ionic'; @@ -19,9 +17,9 @@ export type ScreenshotFn = (fileName: string) => string; export interface TestConfig { direction: Direction; - theme: Theme; palette: Palette; mode: Mode; + theme: Theme; } interface TestUtilities { @@ -31,14 +29,36 @@ interface TestUtilities { } interface TestConfigOption { + /** + * The available options to test against. + * - "ios": Test against iOS theme on iOS mode. + * - "md": Test against Material Design theme on Material Design mode. + * - "ionic-ios": Test against Ionic theme on iOS mode. + * - "ionic-md": Test against Ionic theme on Material Design mode. + * + * If unspecified, tests will run against "ios" and "md". + */ modes?: Mode[]; + /** + * The text direction to test against. + * + * - "ltr": Test against left-to-right direction. + * - "rtl": Test against right-to-left direction. + * + * If unspecified, tests will run against both directions. + */ directions?: Direction[]; - palettes?: Palette[]; /** - * The individual themes (iOS, Material Design and Ionic) to test - * against. If unspecified, defaults iOS and Material Design + * The color palette to test against. + * + * - "light": Test against light theme. + * - "dark": Test against dark theme. + * - "high-contrast": Test against high contrast light theme. + * - "high-contrast-dark": Test against high contrast dark theme. + * + * If unspecified, tests will run against light theme. */ - themes?: Theme[]; + palettes?: Palette[]; } /** @@ -48,9 +68,9 @@ interface TestConfigOption { * each test title is unique. */ const generateTitle = (title: string, config: TestConfig): string => { - const { direction, palette, theme, mode } = config; + const { direction, palette, mode, theme } = config; - if (theme === mode) { + if (theme === 'ios' || theme === 'md') { /** * Fallback to the old test title naming convention * when the theme and mode are the same. @@ -63,10 +83,10 @@ const generateTitle = (title: string, config: TestConfig): string => { * compatibility, we will not include the theme in the test * title if the theme is set to light. */ - return `${title} - ${theme}/${direction}`; + return `${title} - ${mode}/${direction}`; } - return `${title} - ${theme}/${direction}/${palette}`; + return `${title} - ${mode}/${direction}/${palette}`; } return `${title} - ${theme}/${mode}/${direction}/${palette}`; @@ -77,9 +97,9 @@ const generateTitle = (title: string, config: TestConfig): string => { * and a test config. */ const generateScreenshotName = (fileName: string, config: TestConfig): string => { - const { theme, direction, palette, mode } = config; + const { direction, palette, mode, theme } = config; - if (theme === mode) { + if (theme === 'ios' || theme === 'md') { /** * Fallback to the old screenshot naming convention * when the theme and mode are the same. @@ -91,10 +111,10 @@ const generateScreenshotName = (fileName: string, config: TestConfig): string => * compatibility, we will not include the theme in the screenshot * name if the theme is set to light. */ - return `${fileName}-${theme}-${direction}.png`; + return `${fileName}-${mode}-${direction}.png`; } - return `${fileName}-${theme}-${direction}-${palette}.png`; + return `${fileName}-${mode}-${direction}-${palette}.png`; } return `${fileName}-${theme}-${mode}-${direction}-${palette}.png`; @@ -104,7 +124,7 @@ const generateScreenshotName = (fileName: string, config: TestConfig): string => * Given a config generate an array of test variants. */ export const configs = (testConfig: TestConfigOption = DEFAULT_TEST_CONFIG_OPTION): TestUtilities[] => { - const { modes, themes, directions } = testConfig; + const { modes, directions } = testConfig; const configs: TestConfig[] = []; @@ -113,21 +133,24 @@ export const configs = (testConfig: TestConfigOption = DEFAULT_TEST_CONFIG_OPTIO * fall back to the defaults. */ const processedModes = modes ?? DEFAULT_MODES; - const processedTheme = themes ?? DEFAULT_THEMES; const processedDirection = directions ?? DEFAULT_DIRECTIONS; const processedPalette = testConfig.palettes ?? DEFAULT_PALETTES; processedModes.forEach((mode) => { - processedTheme.forEach((theme) => { - if (!isModeValidForTheme(mode, theme)) { - return; - } + if (mode === 'ios' || mode === 'md') { processedDirection.forEach((direction) => { processedPalette.forEach((palette) => { - configs.push({ theme, direction, palette, mode }); + configs.push({ direction, palette, mode, theme: mode }); }); }); - }); + } else { + const [theme, modeName] = mode.split('-'); + processedDirection.forEach((direction) => { + processedPalette.forEach((palette) => { + configs.push({ direction, palette, mode: modeName as Mode, theme: theme as Theme }); + }); + }); + } }); return configs.map((config) => { @@ -140,13 +163,11 @@ export const configs = (testConfig: TestConfigOption = DEFAULT_TEST_CONFIG_OPTIO }; const DEFAULT_MODES: Mode[] = ['ios', 'md']; -const DEFAULT_THEMES: Theme[] = ['ios', 'md']; const DEFAULT_DIRECTIONS: Direction[] = ['ltr', 'rtl']; const DEFAULT_PALETTES: Palette[] = ['light']; const DEFAULT_TEST_CONFIG_OPTION = { modes: DEFAULT_MODES, - themes: DEFAULT_THEMES, directions: DEFAULT_DIRECTIONS, palettes: DEFAULT_PALETTES, }; From eb9f5d6b8bfc60ced66dac394b02b4edbdf462bc Mon Sep 17 00:00:00 2001 From: Sean Perkins Date: Mon, 25 Mar 2024 22:08:14 -0400 Subject: [PATCH 2/5] chore: address feedback --- core/src/utils/test/playwright/generator.ts | 58 ++++++++++++--------- 1 file changed, 34 insertions(+), 24 deletions(-) diff --git a/core/src/utils/test/playwright/generator.ts b/core/src/utils/test/playwright/generator.ts index b1da8a8b59a..676ecd617ad 100644 --- a/core/src/utils/test/playwright/generator.ts +++ b/core/src/utils/test/playwright/generator.ts @@ -70,12 +70,15 @@ interface TestConfigOption { const generateTitle = (title: string, config: TestConfig): string => { const { direction, palette, mode, theme } = config; + /** + * The iOS theme can only be used with the iOS mode, + * and the MD theme can only be used with the MD mode. + * + * This logic enables the fallback behavior for existing tests, + * where we only tested against a mode, which accounted for both + * the theme and mode. + */ if (theme === 'ios' || theme === 'md') { - /** - * Fallback to the old test title naming convention - * when the theme and mode are the same. - */ - if (palette === 'light') { /** * Ionic has many existing tests that existed prior to @@ -85,7 +88,6 @@ const generateTitle = (title: string, config: TestConfig): string => { */ return `${title} - ${mode}/${direction}`; } - return `${title} - ${mode}/${direction}/${palette}`; } @@ -98,12 +100,15 @@ const generateTitle = (title: string, config: TestConfig): string => { */ const generateScreenshotName = (fileName: string, config: TestConfig): string => { const { direction, palette, mode, theme } = config; - + /** + * The iOS theme can only be used with the iOS mode, + * and the MD theme can only be used with the MD mode. + * + * This logic enables the fallback behavior for existing tests, + * where we only tested against a mode, which accounted for both + * the theme and mode. + */ if (theme === 'ios' || theme === 'md') { - /** - * Fallback to the old screenshot naming convention - * when the theme and mode are the same. - */ if (palette === 'light') { /** * Ionic has many existing tests that existed prior to @@ -113,7 +118,6 @@ const generateScreenshotName = (fileName: string, config: TestConfig): string => */ return `${fileName}-${mode}-${direction}.png`; } - return `${fileName}-${mode}-${direction}-${palette}.png`; } @@ -137,20 +141,26 @@ export const configs = (testConfig: TestConfigOption = DEFAULT_TEST_CONFIG_OPTIO const processedPalette = testConfig.palettes ?? DEFAULT_PALETTES; processedModes.forEach((mode) => { - if (mode === 'ios' || mode === 'md') { - processedDirection.forEach((direction) => { - processedPalette.forEach((palette) => { - configs.push({ direction, palette, mode, theme: mode }); - }); - }); + let theme: Theme; + let modeName: Mode; + + if (mode.indexOf('-') === -1) { + /** + * If the mode does not contain a dash, then it is + * either the ios or md configuration, where both + * the mode and theme are the same. + */ + theme = mode as Theme; + modeName = mode as Mode; } else { - const [theme, modeName] = mode.split('-'); - processedDirection.forEach((direction) => { - processedPalette.forEach((palette) => { - configs.push({ direction, palette, mode: modeName as Mode, theme: theme as Theme }); - }); - }); + [theme, modeName] = mode.split('-') as [Theme, Mode]; } + + processedDirection.forEach((direction) => { + processedPalette.forEach((palette) => { + configs.push({ direction, palette, mode: modeName, theme }); + }); + }); }); return configs.map((config) => { From c62e19aac63e8434083b3e0f30db8ee5b8d0420e Mon Sep 17 00:00:00 2001 From: Sean Perkins Date: Tue, 26 Mar 2024 22:32:38 -0400 Subject: [PATCH 3/5] fix: make the code 0.0000000000001% faster SPEED ;) --- core/src/utils/test/playwright/generator.ts | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/core/src/utils/test/playwright/generator.ts b/core/src/utils/test/playwright/generator.ts index 676ecd617ad..9f4f67ec470 100644 --- a/core/src/utils/test/playwright/generator.ts +++ b/core/src/utils/test/playwright/generator.ts @@ -141,24 +141,13 @@ export const configs = (testConfig: TestConfigOption = DEFAULT_TEST_CONFIG_OPTIO const processedPalette = testConfig.palettes ?? DEFAULT_PALETTES; processedModes.forEach((mode) => { - let theme: Theme; - let modeName: Mode; - - if (mode.indexOf('-') === -1) { - /** - * If the mode does not contain a dash, then it is - * either the ios or md configuration, where both - * the mode and theme are the same. - */ - theme = mode as Theme; - modeName = mode as Mode; - } else { - [theme, modeName] = mode.split('-') as [Theme, Mode]; - } + const [themeOrCombinedMode, modeName] = mode.split('-') as [Theme, Mode]; + const parsedTheme = themeOrCombinedMode; + const parsedMode = modeName ?? themeOrCombinedMode; processedDirection.forEach((direction) => { processedPalette.forEach((palette) => { - configs.push({ direction, palette, mode: modeName, theme }); + configs.push({ direction, palette, mode: parsedMode, theme: parsedTheme }); }); }); }); From b9215aefe93b09a6640ae32d9d10493cf1daaecf Mon Sep 17 00:00:00 2001 From: Sean Perkins Date: Wed, 27 Mar 2024 00:01:20 -0400 Subject: [PATCH 4/5] fix: apply test config changes --- core/src/components/button/test/clear/button.e2e.ts | 2 +- core/src/components/button/test/shape/button.e2e.ts | 2 +- core/src/components/button/test/size/button.e2e.ts | 5 ++--- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/core/src/components/button/test/clear/button.e2e.ts b/core/src/components/button/test/clear/button.e2e.ts index 6777705ffd5..a08d95ddc27 100644 --- a/core/src/components/button/test/clear/button.e2e.ts +++ b/core/src/components/button/test/clear/button.e2e.ts @@ -4,7 +4,7 @@ import { configs, test } from '@utils/test/playwright'; /** * Fill="clear" does not render differently based on the direction. */ -configs({ directions: ['ltr'], themes: ['ios', 'md', 'ionic'] }).forEach(({ title, config, screenshot }) => { +configs({ directions: ['ltr'], modes: ['ios', 'md', 'ionic-md'] }).forEach(({ title, config, screenshot }) => { test.describe(title('button: fill: clear'), () => { test('should not have visual regressions', async ({ page }) => { await page.goto(`/src/components/button/test/clear`, config); diff --git a/core/src/components/button/test/shape/button.e2e.ts b/core/src/components/button/test/shape/button.e2e.ts index 40af4fa1960..0682c07dc4c 100644 --- a/core/src/components/button/test/shape/button.e2e.ts +++ b/core/src/components/button/test/shape/button.e2e.ts @@ -74,7 +74,7 @@ configs({ directions: ['ltr'] }).forEach(({ title, screenshot, config }) => { /** * Shape="rectangular" is only available in the Ionic theme. */ -configs({ directions: ['ltr'], themes: ['ionic'] }).forEach(({ title, screenshot, config }) => { +configs({ directions: ['ltr'], modes: ['ionic-md'] }).forEach(({ title, screenshot, config }) => { test.describe(title('button: shape'), () => { test.describe('rectangular', () => { test('should not have visual regressions', async ({ page }) => { diff --git a/core/src/components/button/test/size/button.e2e.ts b/core/src/components/button/test/size/button.e2e.ts index 94b941326ed..c833c77a891 100644 --- a/core/src/components/button/test/size/button.e2e.ts +++ b/core/src/components/button/test/size/button.e2e.ts @@ -1,8 +1,7 @@ import { expect } from '@playwright/test'; import { configs, test } from '@utils/test/playwright'; -// TODO: FW-6077 - Limit this test to just the Ionic theme on MD mode. -configs({ directions: ['ltr'], themes: ['ionic', 'md', 'ios'] }).forEach(({ title, screenshot, config }) => { +configs({ directions: ['ltr'], modes: ['ionic-md', 'md', 'ios'] }).forEach(({ title, screenshot, config }) => { test.describe(title('button: size'), () => { test('should render small buttons', async ({ page }) => { await page.setContent( @@ -65,7 +64,7 @@ configs({ directions: ['ltr'], themes: ['ionic', 'md', 'ios'] }).forEach(({ titl /** * The following tests are specific to the Ionic theme and do not depend on the text direction. */ -configs({ directions: ['ltr'], themes: ['ionic'] }).forEach(({ title, screenshot, config }) => { +configs({ directions: ['ltr'], modes: ['ionic-md'] }).forEach(({ title, screenshot, config }) => { test.describe(title('button: size'), () => { test('should render xsmall buttons', async ({ page }) => { await page.setContent(`X-Small Button`, config); From 656c744f8e47cd0f7e11c5734a37965f93b94918 Mon Sep 17 00:00:00 2001 From: Sean Perkins <13732623+sean-perkins@users.noreply.github.com> Date: Thu, 28 Mar 2024 10:20:41 -0400 Subject: [PATCH 5/5] chore: update palette documentation for test generator Co-authored-by: Liam DeBeasi --- core/src/utils/test/playwright/generator.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/src/utils/test/playwright/generator.ts b/core/src/utils/test/playwright/generator.ts index 9f4f67ec470..3100b5c0d8f 100644 --- a/core/src/utils/test/playwright/generator.ts +++ b/core/src/utils/test/playwright/generator.ts @@ -51,10 +51,10 @@ interface TestConfigOption { /** * The color palette to test against. * - * - "light": Test against light theme. - * - "dark": Test against dark theme. - * - "high-contrast": Test against high contrast light theme. - * - "high-contrast-dark": Test against high contrast dark theme. + * - "light": Test against light palette. + * - "dark": Test against dark palette. + * - "high-contrast": Test against high contrast light palette. + * - "high-contrast-dark": Test against high contrast dark palette. * * If unspecified, tests will run against light theme. */