Skip to content

Commit

Permalink
Add CI tests for experimental code (#9860)
Browse files Browse the repository at this point in the history
### 🤨 WHAT is this pull request doing?

Fixes Shopify/archive-polaris-backlog-2024#1029
Duplicates CI checks for `{features: {polarisSummerEditions2023: true}}`
<details>
<summary>How to run tests in the CLI</summary>
<br />
I implemented this using a <code>SE23</code> environment variable that
can be set to <code>on</code> or <code>off</code>. (I don't use
true/false to avoid confusion with strings vs booleans).

To run tests with `{features: {polarisSummerEditions2023: true}}`:

```
SE23='on' yarn test
```

To run tests with  `{features: {polarisSummerEditions2023: false}}`

```
yarn test
// or
SE23='off' yarn test
```
</details>

<img width="400" alt="Screenshot 2023-08-01 at 1 51 24 PM"
src="https://github.com/Shopify/polaris/assets/20652326/d760c8e1-7cf8-44e2-ba41-c3ba6c7fcf4d">


### 🔨 How tests are fixed

To add these checks, I had to fix all failing tests when
`polarisSummerEditions2023: true`. To do this, I investigated the
failing tests and followed these steps:

1. See if test can be fixed to pass on both without compromising the
purpose of the test
2. If not, duplicate the test
3. Create a `polarisSummerEditions2023 false` describe block at the end
of the file
4. Move the duplicated test there and pass `polarisSummerEditions2023:
false` to `mountWithApp` to force the feature flag on every test run
5. In the original failing test, force `polarisSummerEditions2023: true`
and fix the test

#### Example for <code>CaretDownMinor</code> ->
<code>ChevronDownMinor</code> changes</summary>

```diff
it('is a test that will only pass post se23', () => {
-  const component = mountWithApp(<MyComponent />);
-  expect(component).toContainReactComponent(Icon, {source: CaretDownMinor});
+  const component = mountWithApp(<MyComponent />, {features: {polarisSummerEditions2023: true}});
+  expect(component).toContainReactComponent(Icon, {source: ChevronDownMinor});
});

+ describe('polarisSummerEditions2023 false', () => {
+  it('is a test that will only pass pre se23', () => {
+    const component = mountWithApp(<MyComponent />, {features: {polarisSummerEditions2023: false}});
+      expect(component).toContainReactComponent(Icon, {source: CaretDownMinor});
+  });
```

### 🧽 How these should be cleaned up

When we remove the `polarisSummerEditions2023` feature, we will need
simply:

1. delete all the `polarisSummerEditions2023 false` describe blocks
2. remove the {features: {polarisSummerEditions2023: true} from
`mountWithApp`s

#### Clean up for example above</summary>

```diff
it('is a test that will only pass post se23', () => {
-  const component = mountWithApp(<MyComponent />, {features: {polarisSummerEditions2023: true}});
+. const component = mountWithApp(<MyComponent />);
  expect(component).toContainReactComponent(Icon, {source: ChevronDownMinor});
});

- describe('polarisSummerEditions2023 false', () => {
-   it('is a test that will only pass pre se23', () => {
-     const component = mountWithApp(<MyComponent />, {features: {polarisSummerEditions2023: false}});
-     expect(component).toBeNull();
-   });
```

### How to 🎩

Make sure all CI checks pass 😎
  • Loading branch information
sophschneider authored Aug 8, 2023
1 parent 0cbdbb4 commit af0c9d4
Show file tree
Hide file tree
Showing 22 changed files with 903 additions and 88 deletions.
5 changes: 5 additions & 0 deletions .changeset/poor-forks-switch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/polaris': patch
---

Updated CI tests to account for both polarisSummerEditions2023 beta flag states
1 change: 0 additions & 1 deletion .github/workflows/ci-a11y-vrt-experimental.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ on:
branches:
- main
- next
- beta
paths:
- 'polaris-react/src/**'
- 'polaris-react/playground/**'
Expand Down
1 change: 0 additions & 1 deletion .github/workflows/ci-a11y-vrt.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ on:
branches:
- main
- next
- beta
paths:
- 'polaris-react/src/**'
- 'polaris-react/playground/**'
Expand Down
65 changes: 65 additions & 0 deletions .github/workflows/ci-experimental.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
name: CI experimental

on:
push:
branches:
- main
- next
pull_request:

jobs:
build:
name: 'Validate with Node v${{ matrix.node-version }}'
runs-on: ubuntu-latest
strategy:
matrix:
node-version: ['16.17.0', '18.12.0']
steps:
- name: Checkout branch
uses: actions/checkout@v3
with:
fetch-depth: 2

- name: Free up space on GitHub image
run: |
# Based on the official advice:
# https://github.com/actions/virtual-environments/issues/2840#issuecomment-790492173
sudo rm -rf /usr/share/dotnet
sudo rm -rf /opt/ghc
sudo rm -rf "/usr/local/share/boost"
sudo rm -rf "$AGENT_TOOLSDIRECTORY"
- name: Setup Node with v${{ matrix.node-version }}
uses: actions/setup-node@v3
with:
node-version: ${{ matrix.node-version }}
cache: yarn

- name: Restore cache
uses: actions/cache@v3
with:
path: |
**/.eslintcache
**/.turbo
node_modules/.cache/turbo
polaris.shopify.com/.next/cache
key: ${{ runner.os }}-node${{ matrix.node-version }}-test-experimental-${{ github.sha }}
restore-keys: |
${{ runner.os }}-node${{ matrix.node-version }}-test-experimental-
- name: Install dependencies
run: yarn --frozen-lockfile

- name: Build packages
run: yarn build

- name: Lint
run: yarn lint

- name: Type check
run: yarn type-check

- name: Test
env:
SE23: 'on'
run: yarn test
38 changes: 30 additions & 8 deletions polaris-react/src/components/Badge/tests/Badge.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,19 +34,18 @@ describe('<Badge />', () => {
});

it('does not add pip styles when progress is not provided', () => {
const badge = mountWithApp(<Badge status="attention" />);

expect(badge).not.toContainReactComponent('span', {
className: 'Pip',
const badge = mountWithApp(<Badge status="attention" />, {
features: {polarisSummerEditions2023: true},
});
expect(badge).not.toContainReactComponent(Icon);
});

it('renders with pip styles when progress is provided', () => {
const badge = mountWithApp(<Badge progress="incomplete" />);

expect(badge).toContainReactComponent('span', {
className: 'Pip progressIncomplete',
const badge = mountWithApp(<Badge progress="incomplete" />, {
features: {polarisSummerEditions2023: true},
});

expect(badge).toContainReactComponent(Icon);
});

it('does not render an icon when icon is not provided', () => {
Expand Down Expand Up @@ -172,4 +171,27 @@ describe('<Badge.Pip />', () => {
visuallyHidden: true,
});
});

// se23 -- css pip replaced with icon pip
describe('polarisSummerEditions2023 false', () => {
it('does not add pip styles when progress is not provided', () => {
const badge = mountWithApp(<Badge status="attention" />, {
features: {polarisSummerEditions2023: false},
});

expect(badge).not.toContainReactComponent('span', {
className: 'Pip',
});
});

it('renders with pip styles when progress is provided', () => {
const badge = mountWithApp(<Badge progress="incomplete" />, {
features: {polarisSummerEditions2023: false},
});

expect(badge).toContainReactComponent('span', {
className: 'Pip progressIncomplete',
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ export function BannerExperimental({
return <DefaultBanner {...sharedBannerProps}>{children}</DefaultBanner>;
}

function DefaultBanner({
export function DefaultBanner({
backgroundColor,
textColor,
bannerTitle,
Expand Down Expand Up @@ -171,7 +171,7 @@ function DefaultBanner({
);
}

function InlineIconBanner({
export function InlineIconBanner({
backgroundColor,
bannerIcon,
actionButtons,
Expand Down Expand Up @@ -225,7 +225,7 @@ function InlineIconBanner({
);
}

function WithinContentContainerBanner({
export function WithinContentContainerBanner({
backgroundColor,
textColor,
bannerTitle,
Expand Down
Loading

0 comments on commit af0c9d4

Please sign in to comment.