Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[INFRA] Parallelize Buildkite test job, take 2 #7209

Closed
wants to merge 30 commits into from
Closed
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
1d24b58
Refactoring test Bash script for parallelization.
1Copenut Sep 18, 2023
f5442c7
Splitting test tasks linting and unit testing.
1Copenut Sep 18, 2023
69fd82c
Moved agents declaration into individual steps.
1Copenut Sep 18, 2023
2fb5747
Was missing the double ampersand to run Yarn commands.
1Copenut Sep 18, 2023
0902f9a
Restoring Cypress install to all commands.
1Copenut Sep 18, 2023
7967c91
Adjusting conditional array adds to be one string each.
1Copenut Sep 19, 2023
3c4493c
Removed double quote literals for bash -c command.
1Copenut Sep 19, 2023
4d04d63
Updating all bash strings after successful lint run. Adding one Cypre…
1Copenut Sep 19, 2023
d2a29f4
Adding Cypress runs for React 16 and 17.
1Copenut Sep 19, 2023
2818174
Refactoring test pipeline to switch case for readability.
1Copenut Sep 19, 2023
779c584
Labeling steps for task recognition.
1Copenut Sep 19, 2023
09fe4e8
Updating comment about branch filter.
1Copenut Sep 19, 2023
7b7983c
Updated Cypress screenshots directory path.
1Copenut Sep 20, 2023
d838ce1
Switched commands to use npm so NODE_OPTIONS are available.
1Copenut Sep 20, 2023
fc5da1c
Splitting unit tests on TS and TSX file types.
1Copenut Sep 20, 2023
50211d8
Updating Jest regex to be more inclusive.
1Copenut Sep 20, 2023
25d581f
Quoting Jest regex, updating Cypress React version env variable.
1Copenut Sep 20, 2023
8be998c
Adding commands to package.json for splitting test runs.
1Copenut Sep 20, 2023
3c73d8a
Updated renderHooks to use our React multiple version.
1Copenut Sep 21, 2023
be2d304
Added an act RTL helper to update one test for React 16, 17.
1Copenut Sep 21, 2023
1846d1c
Revert "Added an act RTL helper to update one test for React 16, 17."
1Copenut Sep 21, 2023
30ce21d
Updated test to use multiple version act hook.
1Copenut Sep 21, 2023
0c391c4
Updating Jest tests to run on React 17 or 18.
1Copenut Sep 21, 2023
fc16b78
Standardizing Jest CI commands for React 17, 18.
1Copenut Sep 21, 2023
7ffdcad
Adding Enzyme 16 adapter for unit testing against React 16.
1Copenut Sep 22, 2023
ba2bd12
Updated unit tests and snapshots to pass runs on React 16. Still two …
1Copenut Sep 22, 2023
bd40d77
Updated two unit tests to run correctly on React 16, 17, 18.
1Copenut Sep 22, 2023
e4038a6
Adding final TSX unit test commands to Buildkite test runner.
1Copenut Sep 22, 2023
a428c5b
Merge branch 'main' into infra/parallelize-test-job
1Copenut Sep 25, 2023
b2801ac
Misspelled test definition.
1Copenut Sep 25, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 66 additions & 3 deletions .buildkite/pipelines/pipeline_pull_request_test.yml
Original file line number Diff line number Diff line change
@@ -1,9 +1,72 @@
# 🏠/.buildkite/pipelines/pipeline_pull_request_test.yml

steps:
- agents:
- command: .buildkite/scripts/pipeline_test.sh
label: ":typescript: Linting"
agents:
provider: "gcp"
command: .buildkite/scripts/pipeline_test.sh
if: build.branch != "main" # We're skipping testing commits in main for now to maintain parity with previous Jenkins setup
env:
TEST_TYPE: 'lint'
if: build.branch != "main" # This job is triggered by the combined test and deploy docs for every PR

- command: .buildkite/scripts/pipeline_test.sh
label: ":jest: TS unit tests"
agents:
provider: "gcp"
env:
TEST_TYPE: 'unit:ts'
if: build.branch != "main"
Comment on lines +12 to +18
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once this PR merges, I want to find a way to build our Docker image once, and use a cached version for each command step. This works, but is downloading the Docker image multiple times so it's not as efficient as it could be.

I'm looking at a couple of options for building a safe cached Docker container that could be possibilities after I reduce the image complexity (Puppeteer user and associated libs).


- command: .buildkite/scripts/pipeline_test.sh
label: ":jest: TSX unit tests on React 16"
agents:
provider: "gcp"
env:
TEST_TYPE: 'unit:tsx:16'
if: build.branch != "main"

- command: .buildkite/scripts/pipeline_test.sh
label: ":jest: TSX unit tests on React 17"
agents:
provider: "gcp"
env:
TEST_TYPE: 'unit:tsx:17'
if: build.branch != "main"

- command: .buildkite/scripts/pipeline_test.sh
label: ":jest: TSX unit tests on React 18"
agents:
provider: "gcp"
env:
TEST_TYPE: 'unit:tsx'
if: build.branch != "main"

- command: .buildkite/scripts/pipeline_test.sh
label: ":cypress: Cypress tests on React 16"
agents:
provider: "gcp"
env:
TEST_TYPE: 'cypress:16'
if: build.branch != "main"
artifact_paths:
- "cypress/screenshots/**/*.png"

- command: .buildkite/scripts/pipeline_test.sh
label: ":cypress: Cypress tests on React 17"
agents:
provider: "gcp"
env:
TEST_TYPE: 'cypress:17'
if: build.branch != "main"
artifact_paths:
- "cypress/screenshots/**/*.png"

- command: .buildkite/scripts/pipeline_test.sh
label: ":cypress: Cypress tests on React 18"
agents:
provider: "gcp"
env:
TEST_TYPE: 'cypress:18'
if: build.branch != "main"
artifact_paths:
- "cypress/screenshots/**/*.png"
72 changes: 60 additions & 12 deletions .buildkite/scripts/pipeline_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,63 @@

set -euo pipefail

docker run \
-i --rm \
--env GIT_COMMITTER_NAME=test \
--env GIT_COMMITTER_EMAIL=test \
--env HOME=/tmp \
--user="$(id -u):$(id -g)" \
--volume="$(pwd):/app" \
--workdir=/app \
docker.elastic.co/eui/ci:5.3 \
bash -c "/opt/yarn*/bin/yarn \
&& yarn cypress install \
&& NODE_OPTIONS=\"--max-old-space-size=2048\" npm run test-ci"
DOCKER_OPTIONS=(
-i --rm
--env GIT_COMMITTER_NAME=test
--env GIT_COMMITTER_EMAIL=test
--env HOME=/tmp
--user="$(id -u):$(id -g)"
--volume="$(pwd):/app"
--workdir=/app
docker.elastic.co/eui/ci:5.3
)

case $TEST_TYPE in
lint)
echo "[TASK]: Running linters"
DOCKER_OPTIONS+=(bash -c "/opt/yarn*/bin/yarn && yarn cypress install && NODE_OPTIONS=\"--max-old-space-size=2048\" npm run lint")
;;

unit:ts)
echo "[TASK]: Running .ts unit tests"
DOCKER_OPTIONS+=(bash -c "/opt/yarn*/bin/yarn && yarn cypress install && NODE_OPTIONS=\"--max-old-space-size=2048\" npm run test-unit:ts")
;;

unit:tsx:16)
echo "[TASK]: Running .tsx unit tests"
DOCKER_OPTIONS+=(bash -c "/opt/yarn*/bin/yarn && yarn cypress install && NODE_OPTIONS=\"--max-old-space-size=2048\" npm run test-unit:tsx:16")
;;

unit:tsx:17)
echo "[TASK]: Running .tsx unit tests"
DOCKER_OPTIONS+=(bash -c "/opt/yarn*/bin/yarn && yarn cypress install && NODE_OPTIONS=\"--max-old-space-size=2048\" npm run test-unit:tsx:17")
;;

unit:tsx)
echo "[TASK]: Running .tsx unit tests"
DOCKER_OPTIONS+=(bash -c "/opt/yarn*/bin/yarn && yarn cypress install && NODE_OPTIONS=\"--max-old-space-size=2048\" npm run test-unit:tsx")
;;

cypress:16)
echo "[TASK]: Running Cypress tests against React 16"
DOCKER_OPTIONS+=(bash -c "/opt/yarn*/bin/yarn && yarn cypress install && NODE_OPTIONS=\"--max-old-space-size=2048\" npm run test-cypress:16")
;;

cypress:17)
echo "[TASK]: Running Cypress tests against React 17"
DOCKER_OPTIONS+=(bash -c "/opt/yarn*/bin/yarn && yarn cypress install && NODE_OPTIONS=\"--max-old-space-size=2048\" npm run test-cypress:17")
;;

cypress:18)
echo "[TASK]: Running Cypress tests against React 18"
DOCKER_OPTIONS+=(bash -c "/opt/yarn*/bin/yarn && yarn cypress install && NODE_OPTIONS=\"--max-old-space-size=2048\" npm run test-cypress")
;;

*)
echo "[ERROR]: Unknown task"
echo "Exit code: 1"
exit 1
;;
esac

docker run "${DOCKER_OPTIONS[@]}"
7 changes: 7 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,15 @@
"test": "yarn lint && yarn test-unit",
"test-ci": "yarn test && yarn test-cypress",
"test-unit": "cross-env NODE_ENV=test jest --config ./scripts/jest/config.js",
"test-unit:ts": "cross-env NODE_ENV=test jest --config ./scripts/jest/config.js --testMatch **/*.test.ts **/*.test.js",
"test-unit:tsx": "cross-env NODE_ENV=test jest --config ./scripts/jest/config.js --testMatch **/*.test.tsx",
"test-unit:tsx:17": "cross-env NODE_ENV=test REACT_VERSION=17 jest --config ./scripts/jest/config.js --testMatch **/*.test.tsx",
"test-unit:tsx:16": "cross-env NODE_ENV=test REACT_VERSION=16 jest --config ./scripts/jest/config.js --testMatch **/*.test.tsx",
"test-a11y": "node ./scripts/a11y-testing",
"test-staged": "yarn lint && node scripts/test-staged.js",
"test-cypress": "node ./scripts/cypress",
"test-cypress:17": "node ./scripts/cypress --react-version 17",
"test-cypress:16": "node ./scripts/cypress --react-version 16",
"test-cypress-dev": "node ./scripts/cypress --dev",
"test-cypress-a11y": "node ./scripts/cypress --a11y",
"combine-test-coverage": "sh ./scripts/combine-coverage.sh",
Expand Down Expand Up @@ -186,6 +192,7 @@
"dedent": "^0.7.0",
"dts-generator": "^3.0.0",
"enzyme": "^3.11.0",
"enzyme-adapter-react-16": "^1.15.7",
"enzyme-to-json": "^3.5.0",
"eslint": "^8.41.0",
"eslint-config-prettier": "^8.8.0",
Expand Down
4 changes: 3 additions & 1 deletion scripts/jest/setup/enzyme.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@ if (process.env.REACT_VERSION === '18') {
// is not configured to support act()" errors for now
// TODO: Remove when enzyme tests are replaced with RTL
global.IS_REACT_ACT_ENVIRONMENT = true;
} else {
} else if (process.env.REACT_VERSION === '17') {
Adapter = require('@wojtekmaj/enzyme-adapter-react-17');
} else {
Adapter = require('enzyme-adapter-react-16');
Comment on lines +16 to +17
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, what tests were failing when this wasn't being used?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A large number of unit tests were failing, over a third, spread across multiple suites.

}

configure({ adapter: new Adapter() });
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ exports[`EuiCollapsibleNavBeta responsive behavior collapses from a push flyout

exports[`EuiCollapsibleNavBeta responsive behavior makes the overlay flyout full width once the screen is smaller than 1.5x the flyout width 1`] = `
<body
class="euiBody--hasFlyout"
class=""
style="padding-left: 0px;"
>
<div>
Expand All @@ -147,9 +147,9 @@ exports[`EuiCollapsibleNavBeta responsive behavior makes the overlay flyout full
>
<button
aria-controls="generated-id_euiCollapsibleNav"
aria-expanded="true"
aria-label="Toggle navigation closed"
aria-pressed="true"
aria-expanded="false"
aria-label="Toggle navigation open"
aria-pressed="false"
class="euiButtonIcon euiCollapsibleNavButton emotion-euiButtonIcon-s-empty-text-euiCollapsibleNavButton"
data-test-subj="euiCollapsibleNavButton"
type="button"
Expand All @@ -158,49 +158,10 @@ exports[`EuiCollapsibleNavBeta responsive behavior makes the overlay flyout full
aria-hidden="true"
class="euiButtonIcon__icon"
color="inherit"
data-euiicon-type="cross"
data-euiicon-type="menu"
/>
</button>
</div>
</div>
<div
class="euiOverlayMask emotion-euiOverlayMask-belowHeader"
data-euiportal="true"
data-relative-to-header="below"
>
<div
data-focus-guard="true"
style="width: 1px; height: 0px; padding: 0px; overflow: hidden; position: fixed; top: 1px; left: 1px;"
tabindex="0"
/>
<div
data-focus-lock-disabled="false"
>
<nav
aria-describedby="generated-id"
aria-label="Site menu"
class="euiFlyout euiCollapsibleNav euiCollapsibleNavBeta emotion-euiFlyout-none-noMaxWidth-overlay-left-euiCollapsibleNavBeta-left-isOverlayFullWidth"
data-autofocus="true"
id="generated-id_euiCollapsibleNav"
role="dialog"
style="inline-size: 100%;"
tabindex="0"
>
<p
class="emotion-euiScreenReaderOnly"
id="generated-id"
>
You are in a modal dialog. Press Escape or tap/click outside the dialog on the shadowed overlay to close.

</p>
Nav content
</nav>
</div>
<div
data-focus-guard="true"
style="width: 1px; height: 0px; padding: 0px; overflow: hidden; position: fixed; top: 1px; left: 1px;"
tabindex="0"
/>
</div>
</body>
`;
40 changes: 29 additions & 11 deletions src/components/collapsible_nav_beta/collapsible_nav_beta.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@
import React from 'react';
import { fireEvent } from '@testing-library/react';
import { render } from '../../test/rtl';
import { shouldRenderCustomStyles } from '../../test/internal';
import {
shouldRenderCustomStyles,
testOnReactVersion,
} from '../../test/internal';
import { requiredProps } from '../../test';

import { EuiCollapsibleNavBeta } from './collapsible_nav_beta';
Expand Down Expand Up @@ -70,21 +73,36 @@ describe('EuiCollapsibleNavBeta', () => {
window.dispatchEvent(new Event('resize'));
};

it('collapses from a push flyout to an overlay flyout once the screen is smaller than 3x the flyout width', () => {
mockWindowResize(600);
const { baseElement } = render(
<EuiCollapsibleNavBeta>Nav content</EuiCollapsibleNavBeta>
);
expect(baseElement).toMatchSnapshot();
});
testOnReactVersion(['18'])(
'collapses from a push flyout to an overlay flyout once the screen is smaller than 3x the flyout width',
() => {
mockWindowResize(600);
const { baseElement } = render(
<EuiCollapsibleNavBeta>Nav content</EuiCollapsibleNavBeta>
);
expect(baseElement).toMatchSnapshot();
}
);

testOnReactVersion(['18'])(
'makes the overlay flyout full width once the screen is smaller than 1.5x the flyout width',
() => {
mockWindowResize(320);
const { baseElement } = render(
<EuiCollapsibleNavBeta>Nav content</EuiCollapsibleNavBeta>
);
expect(baseElement).toMatchSnapshot();
}
);

it('makes the overlay flyout full width once the screen is smaller than 1.5x the flyout width', () => {
it('dauptes the overlay controls once the screen is smaller than 1.5x the flyout width', () => {
mockWindowResize(320);
const { baseElement, getByTestSubject } = render(
const { baseElement, getByLabelText, getByTestSubject } = render(
<EuiCollapsibleNavBeta>Nav content</EuiCollapsibleNavBeta>
);
expect(getByLabelText('Toggle navigation open')).toBeInTheDocument();
fireEvent.click(getByTestSubject('euiCollapsibleNavButton'));
expect(baseElement).toMatchSnapshot();
expect(getByLabelText('Toggle navigation closed')).toBeInTheDocument();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I refactored this to two React 18 snapshot tests and an actual unit test for mobile adaptive behavior on Cee's recommendation in a Slack chat. The prior snapshot tests were rewriting themselves when I ran them in React 16, so this felt a better approach with respect to test coverage.


// onClose testing
expect(
Expand Down
8 changes: 3 additions & 5 deletions src/components/combo_box/combo_box.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -427,11 +427,9 @@ describe('behavior', () => {
});

act(() => {
const searchInputNode = searchInput.getDOMNode();
// React doesn't support `focusout` so we have to manually trigger it
searchInputNode.dispatchEvent(
new FocusEvent('focusout', { bubbles: true })
);
// React 16 failed on the previous `searchInputNode.dispatchEvent`
// call that mocked a FocusEvent 'focusout'
searchInput.simulate('blur');
});

expect(onCreateOptionHandler).toHaveBeenCalledTimes(1);
Expand Down
Loading
Loading