
Start your review here 👉 [docs/README.md](https://github.com/ionic-team/ionic-framework/blob/FW-6107/docs/README.md) ## What is the current behavior? Documentation files with information on how to contribute, component implementations, testing, etc. are scattered throughout various folders in this repository. ## What is the new behavior? Consolidates the documentation files into a root `docs/` directory for easier discovery and organization. `/docs` tree: ``` ├── _config.yml ├── component-guide.md ├── CONTRIBUTING.md ├── README.md ├── sass-guidelines.md ├── angular │ ├── README.md │ └── testing.md ├── core │ ├── README.md │ └── testing │ ├── README.md │ ├── api.md │ ├── best-practices.md │ ├── preview-changes.md │ └── usage-instructions.md ├── react │ ├── README.md │ └── testing.md ├── react-router │ ├── README.md │ └── testing.md ├── vue │ ├── README.md │ └── testing.md └── vue-router ├── README.md └── testing.md ``` **Migrates the following:** | Previous Location | New Location | | ----------------------------------------------------------- | ----------------------------------------- | | `.github/COMPONENT-GUIDE.md` | `docs/component-guide.md` | | `.github/CONTRIBUTING.md` | `docs/CONTRIBUTING.md` | | `core/scripts/README.md` | `docs/core/testing/preview-changes.md` | | `core/src/utils/test/playwright/docs/api.md` | `docs/core/testing/api.md` | | `core/src/utils/test/playwright/docs/best-practices.md` | `docs/core/testing/best-practices.md` | | `core/src/utils/test/playwright/docs/README.md` | `docs/core/testing/README.md` | | `core/src/utils/test/playwright/docs/usage-instructions.md` | `docs/core/testing/usage-instructions.md` | | `packages/angular/test/README.md` | `docs/angular/testing.md` | | `packages/react-router/test/README.md` | `docs/react-router/testing.md` | | `packages/react/test/README.md` | `docs/react/testing.md` | | `packages/react/test/base/README.md` | `docs/react/testing.md` | | `packages/vue/test/README.md` | `docs/vue/testing.md` | **Adds the following:** | File | Description | | ----------------------------- | ----------------------------------------------------------------------- | | `docs/sass-guidelines.md` | Sass Variable guidelines taken from `ionic-framework-design-documents` | | `docs/README.md` | Entry file that should link to all other files | | `docs/_config.yml` | Config file for use with GitHub pages | | `docs/core/README.md` | Description of core, links to contributing and testing | | `docs/angular/README.md` | Description of angular, links to contributing and testing | | `docs/react/README.md` | Description of react, links to contributing and testing | | `docs/react-router/README.md` | Description of react-router, links to contributing and testing | | `docs/vue/README.md` | Description of vue, links to contributing and testing | | `docs/vue-router/README.md` | Description of vue-router, links to contributing and testing | | `docs/vue-router/testing.md` | Testing file for vue-router, populated from vue-router's main README | **Does not** add any files for `angular-server`. This is because the README is essentially empty and there is no testing in that directory. I can add blank files if we want to have something to add to later. **Does not** migrate the content of the packages' root `README.md` files. These files are used for their npm package descriptions so we should not edit them. ## Hosting Documentation We can (and should) host these files using GitHub Pages. I have duplicated them in a personal repository to see how this would look: [docs-consolidation](https://brandyscarney.github.io/docs-consolidation/). Doing so will require some formatting fixes (see [Sass Guidelines](https://brandyscarney.github.io/docs-consolidation/sass-guidelines.html#-reusable-values)) so I did not publish them now but we can easily enable GitHub pages by toggling a setting in this repository. ## Other information - Verify that no documentation files were missed in the migration - You can use these commands to search for `*.md` files in a directory: - `find core/src -type f -name "*.md" -print` - `find packages/angular -type f -name "*.md" -not -path "**/node_modules/*" -print` - I did add some redirect links in some of the existing markdown files so they might still exist for that reason - We should probably break up the contributing + component guide documentation into smaller files, such as including best practices, but I wanted to get everything in the same place first - The contributing has sections on each of the packages that we could move to that package's docs folder: https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#core --------- Co-authored-by: Maria Hutt <thetaPC@users.noreply.github.com>
12 KiB
Best Practices
This guide details best practices that should be followed when writing E2E tests.
Table of Contents
- Use the customized
test
function - Break up test directories per-feature
- Follow the file format
- Ensure each component has a
basic
test directory with anindex.html
file - Use
test.describe
blocks to describe groups of tests - Place
configs
generator outside thetest.describe
block - Test rendering and functionality in separate
test.describe
blocks - Break up large or slow-running tests across multiple files
- Use standard viewport sizes
- Avoid using screenshots as a way of verifying functionality
- Avoid tests that compare computed values
- Test for positive and negative cases
- Start your test with the configuration or layout in place if possible
- Place your test closest to the fix or feature
- Account for different locales when writing tests
Use the customized `test` function
Do not import the test
fixture from @playwright/test
. Instead, use the test
fixture defined in @utils/test/playwright
. This is a custom Playwright fixture that extends the built-in test
fixture and has logic to wait for the Stencil app to load before proceeding with the test. If you do not use this test fixture, your screenshots will likely be blank.
Since this fixture extends the built in test
fixture, all of the normal methods found in the Playwright documentation still apply.
This is the only custom fixture you need. All of the other fixtures such as expect
can be imported from @playwright/test
.
Note: @utils
is an alias defined in tsconfig.json
that points to /src/utils
. This lets us avoid doing ../../../../
if we are several folders deep when importing.
Break up test directories per-feature
Tests should be broken up per-feature. This makes it easy for team members to quickly find tests for a particular feature. Additionally, the names of test directories should use kebab-case.
basic/component.e2e.ts
feature-a/component.e2e.ts
feature-b/component.e2e.ts
feature-c/component.e2e.ts
Example:
The first-day-of-week
directory has all the tests for the firstDayOfWeek
property on ion-datetime
, and the color
directory has all the tests for the color
property usage.
Some features can be combined together, and so it is acceptable in this instance to test features together in a single directory.
Follow the file format
E2E test files should follow this format:
[component name].e2e.ts
It is recommended to have one E2E test file per directory.
Example:
/basic
button.e2e.ts
/anchor
button.e2e.ts
/form
button.e2e.ts
In the event you need multiple E2E files per directory, add a modifier to the file that makes it unique.
Example:
/basic
modal-controller.e2e.ts // E2E tests for ion-modal via modalController
modal-inline.e2e.ts // E2E tests for ion-modal via <ion-modal>
Ensure each component has a `basic` test directory with an `index.html` file
At a minimum, each component with a tests
directory must also have a basic
directory with an index.html
file. This is done so team members can easily paste usage examples to test out when developing or reviewing PRs. The basic
directory may have E2E tests, but they should be limited to testing the default (or basic) behavior of a component.
Use `test.describe` blocks to describe groups of tests
Each E2E test file should have at least 1 test.describe
block which defines the component and the feature you are testing.
Example:
// src/components/button/test/basic/button.e2e.ts
import { configs, test } from '@utils/test/playwright';
configs().forEach(({ title }) => {
test.describe(title('button: disabled state'), () => {
...
});
});
There should be one or more test
blocks which have individual tests. The test name describes what the test should do.
Example:
// src/components/button/test/basic/button.e2e.ts
import { configs, test } from '@utils/test/playwright';
configs().forEach(({ title }) => {
test.describe(title('button: disabled state'), () => {
test('should not have any visual regressions', async ({ page }) => {
...
});
});
});
Place `configs` generator outside the `test.describe` block
The configs()
generator should be done outside of the test.describe
block. The benefit of this is it lets you use test.beforeEach
to run a common page.goto
or page.setContent
while passing in the correct config. It also lets you use the title
function just on the test.describe
block instead of each test
inside the block.
❌ Incorrect
import { configs test } from '@utils/test/playwright';
test.describe('button: disabled state', () => {
configs().forEach(({ config, title }) => {
/**
* This will generate a `test.beforeEach` for each test
* config, and all of the generated `test.beforeEach` blocks
* will be run on each test.
*/
test.beforeEach(async ({ page }) => {
await page.goto('/src/components/button/test', config);
});
test(title('should not have any visual regressions'), async ({ page }) => {
...
});
});
});
✅ Correct
import { configs test } from '@utils/test/playwright';
configs().forEach(({ config, title }) => {
test.describe(title('button: disabled state'), () => {
/**
* This will generate one `test.beforeEach` for
* each `test.describe` block rather than for
* each `test` inside of the `test.describe` block.
*/
test.beforeEach(async ({ page }) => {
await page.goto('/src/components/button/test', config);
});
/**
* Test title is still unique because of our use of title()
* on the test.describe block.
*/
test('should not have any visual regressions', async ({ page }) => {
...
});
});
});
Test rendering and functionality in separate `test.describe` blocks
Avoid mixing tests that take screenshots with tests that check functionality. These types of tests often have different requirements that can make a single test.describe
block hard to understand. For example, a screenshot test might check both iOS and MD modes with LTR and RTL text directions, but a functionality test may not need that if the functionality is consistent across modes and directions.
If using multiple test.describe
blocks creates a large test file, consider breaking up these tests across multiple files.
Break up large or slow-running tests across multiple files
Tests are distributed across many test runners on continuous integration (CI) to improve performance. However, Playwright distributes tests based on test file, not individual test. This means test files that are particularly slow will negatively impact the overall CI performance.
Use standard viewport sizes
By default, we run tests on mobile viewports only (think iPhone sized viewports). However, there are some components that have different layouts on tablet viewports. Two examples are ion-split-pane
and the card variant of ion-modal
.
For this scenario, developers must write tests that target the tablet viewport. This can be done by using page.setViewportSize. The Playwright test utils directory also contains a Viewports
constant which contains some common viewport presets. Developers should feel free to add new viewports to this as is applicable.
Example:
import { configs, test, Viewports } from '@utils/test/playwright';
configs().forEach(({ config, title }) => {
test.describe(title('thing: rendering'), () => {
test('it should do a thing on tablet viewports', async ({ page }) => {
await page.setViewportSize(Viewports.tablet.portrait);
...
// test logic goes here
});
});
});
Avoid using screenshots as a way of verifying functionality
Screenshots are great for verifying visual differences, but using it to verify functionality can lead to issues. Instead, try testing for the existence of elements in the DOM, events emitted, etc.
❌ Incorrect
This test ensures that the isOpen
property opens a modal when true
. It does not verify that the inner contents of a modal are rendered as expected as that is outside the scope of this test. As a result, using a screenshot would not be appropriate here.
configs().forEach(({ config, title }) => {
test.describe(title('modal: rendering') => {
test('it should open a modal', async ({ page }) => {
await page.setContent('<ion-modal is-open="true">...</ion-modal>', config);
await expect(page).toHaveScreenshot(screenshot('modal-open'));
});
});
});
✅ Correct
Instead, we can use a toBeVisible
assertion to verify that isOpen
does present a modal when true
.
configs().forEach(({ config, title }) => {
test.describe(title('modal: rendering') => {
test('it should open a modal', async ({ page }) => {
await page.setContent('<ion-modal is-open="true">...</ion-modal>', config);
const modal = page.locator('ion-modal');
await expect(modal).toBeVisible();
});
});
});
Avoid tests that compare computed values
All browsers render web content in slightly different manners. Instead of testing computed values such as exact pixel values, screenshots are a great way to ensure that elements are being rendered in a consistent manner across browsers.
Test for positive and negative cases
It’s important to test that your code works when the API is used as intended, but what happens if someone makes a mistake? While some errors are fine, incorrect usage should not cause catastrophic failures such as data loss. While TypeScript helps catch incorrect usages, not everyone uses TypeScript in Ionic apps. Additionally, developers will sometimes typecast as any
.
Start your test with the configuration or layout in place if possible
This allows tests to remain fast on CI as we can focus on the test itself instead of navigating to the state where the test begins. For example, a simple scrollIntoViewIfNeeded in Playwright can take around 300ms on CI. Since we run a single test for multiple configurations, that 300ms can add up quickly. Consider setting up your test in a way that the element you want to test is already in view when the test starts.
Place your test closest to the fix or feature
Tests should be placed closest to where the fix or feature was implemented. This means that if a fix was written for ion-button
, then the test should be placed in src/components/button/tests
.
Account for different locales when writing tests
Tests ran on CI may not run on the same locale as your local machine. It's always a good idea to apply locale considerations to components that support it, when writing tests (i.e. ion-datetime
should specify locale="en-US"
).