Skip to content

Commit

Permalink
chore(playwright): combine test configs for themes and modes (#29206)
Browse files Browse the repository at this point in the history
Issue number: N/A

---------

<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->

<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->

## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->

Developers are not able to easily add the "ionic" theme to existing test
cases, without running the test against both iOS and MD mode:

```ts
configs({ themes: ['ios', 'md', 'ionic'] })
// Generates 4 test cases
// - iOS theme on iOS mode
// - MD theme on MD mode
// - Ionic theme on iOS mode
// - Ionic theme on MD mode
```

With the separation of `mode` into look and feel, the majority of test
cases do not require testing the mode behavior and instead only need to
test the visual theme that is applied to the component.

## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->

- Removes the `themes` option from the `configs` test generator object.

```ts
configs({ modes: ['ios', 'md', 'ionic-md'] })
```

- Combines `theme` and `mode` into the existing `modes` test generator
object
- The new options are `ionic-ios` and `ionic-md`, to run the Ionic theme
against the respective mode.
- This path was preferred to avoid deprecating and migrating all
existing tests.

## Does this introduce a breaking change?

- [ ] Yes
- [x] No

<!--
  If this introduces a breaking change:
1. Describe the impact and migration path for existing applications
below.
  2. Update the BREAKING.md file with the breaking change.
3. Add "BREAKING CHANGE: [...]" to the commit description when merging.
See
https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#footer
for more information.
-->


## Other information

<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->

I do not have a strong preference on the semantics of `ionic-ios` vs.
`ios-ionic` (theme first vs. mode first). If anyone has an opinion or
alternative suggestion, please add a comment.

---------

Co-authored-by: Liam DeBeasi <liamdebeasi@users.noreply.github.com>
  • Loading branch information
sean-perkins and liamdebeasi committed Mar 28, 2024
1 parent 6d6fd4a commit 5234224
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 45 deletions.
2 changes: 1 addition & 1 deletion core/src/components/button/test/clear/button.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion core/src/components/button/test/shape/button.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 }) => {
Expand Down
5 changes: 2 additions & 3 deletions core/src/components/button/test/size/button.e2e.ts
Original file line number Diff line number Diff line change
@@ -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(
Expand Down Expand Up @@ -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(`<ion-button size="xsmall" fill="solid">X-Small Button</ion-button>`, config);
Expand Down
100 changes: 60 additions & 40 deletions core/src/utils/test/playwright/generator.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -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 {
Expand All @@ -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 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.
*/
themes?: Theme[];
palettes?: Palette[];
}

/**
Expand All @@ -48,25 +68,27 @@ interface TestConfigOption {
* each test title is unique.
*/
const generateTitle = (title: string, config: TestConfig): string => {
const { direction, palette, theme, mode } = config;

if (theme === mode) {
/**
* Fallback to the old test title naming convention
* when the theme and mode are the same.
*/
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') {
if (palette === 'light') {
/**
* Ionic has many existing tests that existed prior to
* the introduction of theme testing. To maintain backwards
* 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}`;
Expand All @@ -77,24 +99,26 @@ const generateTitle = (title: string, config: TestConfig): string => {
* and a test config.
*/
const generateScreenshotName = (fileName: string, config: TestConfig): string => {
const { theme, direction, palette, mode } = config;

if (theme === mode) {
/**
* Fallback to the old screenshot naming convention
* when the theme and mode are the same.
*/
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') {
if (palette === 'light') {
/**
* Ionic has many existing tests that existed prior to
* the introduction of theme testing. To maintain backwards
* 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`;
Expand All @@ -104,7 +128,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[] = [];

Expand All @@ -113,19 +137,17 @@ 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;
}
processedDirection.forEach((direction) => {
processedPalette.forEach((palette) => {
configs.push({ theme, direction, palette, 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: parsedMode, theme: parsedTheme });
});
});
});
Expand All @@ -140,13 +162,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,
};

0 comments on commit 5234224

Please sign in to comment.