Skip to content

Commit

Permalink
Upgrades jest to v27, simplifies GitHub workflow
Browse files Browse the repository at this point in the history
* Upgrades `jest` from v26.4.2 to v27.5.1.
  * [CHANGELOG](https://github.com/facebook/jest/blob/v27.5.1/CHANGELOG.md)
  * v27 introduced breaking changes that had to be addressed.
* Removes caching from GitHub test workflow to simplify.
* Removes "bad apples" check from GitHub workflow.
* Runs unit tests in band to reduce execution time from ~90
  minutes to ~11 minutes.
* Adds ci-specific scripts to eliminate duplication of commands.
  * `yarn test:jest:ci` and `yarn test:jest_integration:ci` are now used
    in the workflow.
* Upgrades testing-related dependencies, removes unused dependency
`babel-plugin-istanbul`, and replaces `jest-environment-jsdom-thirteen`
with the built-in `jest-environment-jsdom`.
* Skips a few flaky tests that need require additional investigation.

Resolves #1231

Signed-off-by: Tommy Markley <[email protected]>
  • Loading branch information
Tommy Markley committed Mar 2, 2022
1 parent 7cb8297 commit 8da108a
Show file tree
Hide file tree
Showing 62 changed files with 1,954 additions and 2,599 deletions.
10 changes: 0 additions & 10 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -536,16 +536,6 @@ module.exports = {
},
},

/**
* Jest specific rules
*/
{
files: ['**/*.test.{js,mjs,ts,tsx}'],
rules: {
'jest/valid-describe': 'error',
},
},

/**
* Harden specific rules
*/
Expand Down
117 changes: 4 additions & 113 deletions .github/workflows/pr_check_workflow.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,108 +26,36 @@ jobs:
runs-on: ubuntu-latest
name: Build and Verify
steps:
# Access a cache of set results from a previous run of the job
# This is to prevent re-running steps that were already successful since it is not native to github actions
# Can be used to verify flaky steps with reduced times
- name: Restore the cached run
uses: actions/cache@v2
with:
path: |
job_successful
linter_results
unit_tests_results
integration_tests_results
key: ${{ github.run_id }}-${{ github.job }}-${{ github.sha }}
restore-keys: |
${{ github.run_id }}-${{ github.job }}-${{ github.sha }}
- name: Get if previous job was successful
id: job_successful
run: cat job_successful 2>/dev/null || echo 'false'

- name: Get the previous linter results
id: linter_results
run: cat linter_results 2>/dev/null || echo 'default'

- name: Get the previous unit tests results
id: unit_tests_results
run: cat unit_tests_results 2>/dev/null || echo 'default'

- name: Get the previous integration tests results
id: integration_tests_results
run: cat integration_tests_results 2>/dev/null || echo 'default'

- name: Checkout code
if: steps.job_successful.outputs.job_successful != 'true'
uses: actions/checkout@v2

- name: Setup Node
if: steps.job_successful.outputs.job_successful != 'true'
uses: actions/setup-node@v2
with:
node-version-file: ".nvmrc"
registry-url: 'https://registry.npmjs.org'

- name: Setup Yarn
if: steps.job_successful.outputs.job_successful != 'true'
run: |
npm uninstall -g yarn
npm i -g [email protected]
- name: Run bootstrap
if: steps.job_successful.outputs.job_successful != 'true'
run: yarn osd bootstrap

- name: Run linter
if: steps.linter_results.outputs.linter_results != 'success'
id: linter
run: yarn lint

# Runs unit tests while limiting workers because github actions will spawn more than it can handle and crash
# Continues on error but will create a comment on the pull request if this step failed.
- name: Run unit tests
if: steps.unit_tests_results.outputs.unit_tests_results != 'success'
id: unit-tests
continue-on-error: true
run: node scripts/jest --ci --colors --maxWorkers=10
env:
SKIP_BAD_APPLES: true

- run: echo Unit tests completed unsuccessfully. However, unit tests are inconsistent on the CI so please verify locally with `yarn test:jest`.
if: steps.unit_tests_results.outputs.unit_tests_results != 'success' && steps.unit-tests.outcome != 'success'

# TODO: This gets rejected, we need approval to add this
# - name: Add comment if unit tests did not succeed
# if: steps.unit_tests_results.outputs.unit_tests_results != 'success' && steps.unit-tests.outcome != 'success'
# uses: actions/github-script@v5
# with:
# github-token: ${{ secrets.GITHUB_TOKEN }}
# script: |
# github.rest.issues.createComment({
# issue_number: context.issue.number,
# owner: context.repo.owner,
# repo: context.repo.repo,
# body: 'Unit tests completed unsuccessfully. However, unit tests are inconsistent on the CI so please verify locally with `yarn test:jest`.'
# })
run: yarn test:jest:ci

- name: Run integration tests
if: steps.integration_tests_results.outputs.integration_tests_results != 'success'
id: integration-tests
run: node scripts/jest_integration --ci --colors --max-old-space-size=5120

# Set cache if linter, unit tests, and integration tests were successful then the job will be marked successful
# Sets individual results to empower re-runs of the same build without re-running successful steps.
- if: |
(steps.linter.outcome == 'success' || steps.linter.outcome == 'skipped') &&
(steps.unit-tests.outcome == 'success' || steps.unit-tests.outcome == 'skipped') &&
(steps.integration-tests.outcome == 'success' || steps.integration-tests.outcome == 'skipped')
run: echo "::set-output name=job_successful::true" > job_successful
- if: steps.linter.outcome == 'success' || steps.linter.outcome == 'skipped'
run: echo "::set-output name=linter_results::success" > linter_results
- if: steps.unit-tests.outcome == 'success' || steps.unit-tests.outcome == 'skipped'
run: echo "::set-output name=unit_tests_results::success" > unit_tests_results
- if: steps.integration-tests.outcome == 'success' || steps.integration-tests.outcome == 'skipped'
run: echo "::set-output name=integration_tests_results::success" > integration_tests_results
run: yarn test:jest_integration:ci

functional-tests:
needs: [ build-lint-test ]
runs-on: ubuntu-latest
Expand All @@ -138,76 +66,39 @@ jobs:
steps:
- run: echo Running functional tests for ciGroup${{ matrix.group }}

# Access a cache of set results from a previous run of the job
# This is to prevent re-running a CI group that was already successful since it is not native to github actions
# Can be used to verify flaky steps with reduced times
- name: Restore the cached run
uses: actions/cache@v2
with:
path: |
ftr_tests_results
key: ${{ github.run_id }}-${{ github.job }}-${{ matrix.group }}-${{ github.sha }}
restore-keys: |
${{ github.run_id }}-${{ github.job }}-${{ matrix.group }}-${{ github.sha }}
- name: Get the cached tests results
id: ftr_tests_results
run: cat ftr_tests_results 2>/dev/null || echo 'default'

- name: Checkout code
if: steps.ftr_tests_results.outputs.ftr_tests_results != 'success'
uses: actions/checkout@v2

- name: Setup Node
if: steps.ftr_tests_results.outputs.ftr_tests_results != 'success'
uses: actions/setup-node@v2
with:
node-version-file: ".nvmrc"
registry-url: 'https://registry.npmjs.org'

- name: Setup Yarn
if: steps.ftr_tests_results.outputs.ftr_tests_results != 'success'
run: |
npm uninstall -g yarn
npm i -g [email protected]
- name: Get cache path
if: steps.ftr_tests_results.outputs.ftr_tests_results != 'success'
id: cache-path
run: echo "::set-output name=CACHE_DIR::$(yarn cache dir)"

- name: Setup cache
if: steps.ftr_tests_results.outputs.ftr_tests_results != 'success'
uses: actions/cache@v2
with:
path: ${{ steps.cache-path.outputs.CACHE_DIR }}
key: ${{ runner.os }}-yarn-${{ env.CACHE_NAME }}-${{ hashFiles('**/yarn.lock') }}
restore-keys: |
${{ runner.os }}-yarn-${{ env.CACHE_NAME }}-
${{ runner.os }}-yarn-
${{ runner.os }}-
# github virtual env is the latest chrome
- name: Setup chromedriver
if: steps.ftr_tests_results.outputs.ftr_tests_results != 'success'
run: yarn add --dev [email protected]

- name: Run bootstrap
if: steps.ftr_tests_results.outputs.ftr_tests_results != 'success'
run: yarn osd bootstrap

- name: Build plugins
if: steps.ftr_tests_results.outputs.ftr_tests_results != 'success'
run: node scripts/build_opensearch_dashboards_platform_plugins --no-examples --workers 10

- if: steps.ftr_tests_results.outputs.ftr_tests_results != 'success'
- name: Run CI test group ${{ matrix.group }}
id: ftr-tests
run: node scripts/functional_tests.js --config test/functional/config.js --include ciGroup${{ matrix.group }}
env:
CI_GROUP: ciGroup${{ matrix.group }}
CI_PARALLEL_PROCESS_NUMBER: ciGroup${{ matrix.group }}
JOB: ci${{ matrix.group }}
CACHE_DIR: ciGroup${{ matrix.group }}

- if: steps.ftr-tests.outcome == 'success' || steps.ftr-tests.outcome == 'skipped'
run: echo "::set-output name=ftr_tests_results::success" > ftr_tests_results
41 changes: 20 additions & 21 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@
"test": "grunt test",
"test:bwc": "./scripts/bwctest-osd.sh",
"test:jest": "node scripts/jest",
"test:jest:ci": "node scripts/jest --ci --colors --runInBand",
"test:jest_integration": "node scripts/jest_integration",
"test:jest_integration:ci": "node scripts/jest_integration --ci --colors --max-old-space-size=5120",
"test:mocha": "node scripts/mocha",
"test:mocha:coverage": "grunt test:mochaCoverage",
"test:ftr": "node scripts/functional_tests",
Expand Down Expand Up @@ -135,7 +137,7 @@
"@types/yauzl": "^2.9.1",
"JSONStream": "1.3.5",
"abortcontroller-polyfill": "^1.4.0",
"angular": "^1.8.0",
"angular": "^1.8.2",
"angular-elastic": "^2.5.1",
"angular-sanitize": "^1.8.0",
"bluebird": "3.5.5",
Expand Down Expand Up @@ -233,12 +235,12 @@
"@osd/utility-types": "1.0.0",
"@percy/cli": "^1.0.0-beta.74",
"@percy/sdk-utils": "^1.0.0-beta.74",
"@testing-library/dom": "^7.24.2",
"@testing-library/jest-dom": "^5.11.4",
"@testing-library/react": "^11.0.4",
"@testing-library/react-hooks": "^3.4.1",
"@types/angular": "^1.6.56",
"@types/angular-mocks": "^1.7.0",
"@testing-library/dom": "^8.11.3",
"@testing-library/jest-dom": "^5.16.2",
"@testing-library/react": "^12.1.2",
"@testing-library/react-hooks": "^7.0.2",
"@types/angular": "^1.8.4",
"@types/angular-mocks": "^1.7.1",
"@types/archiver": "^3.1.0",
"@types/babel__core": "^7.1.17",
"@types/bluebird": "^3.1.1",
Expand Down Expand Up @@ -267,7 +269,7 @@
"@types/has-ansi": "^3.0.0",
"@types/history": "^4.7.3",
"@types/hjson": "^2.4.2",
"@types/jest": "^26.0.14",
"@types/jest": "^27.4.0",
"@types/joi": "^13.4.2",
"@types/jquery": "^3.3.31",
"@types/js-yaml": "^3.11.1",
Expand Down Expand Up @@ -304,12 +306,11 @@
"@types/sinon": "^7.0.13",
"@types/strip-ansi": "^5.2.1",
"@types/styled-components": "^5.1.19",
"@types/supertest": "^2.0.5",
"@types/supertest": "^2.0.11",
"@types/supertest-as-promised": "^2.0.38",
"@types/tapable": "^1.0.6",
"@types/tar": "^4.0.3",
"@types/testing-library__jest-dom": "^5.9.3",
"@types/testing-library__react-hooks": "^3.4.0",
"@types/testing-library__jest-dom": "^5.14.2",
"@types/tough-cookie": "^4.0.1",
"@types/type-detect": "^4.0.1",
"@types/uuid": "^3.4.4",
Expand All @@ -321,15 +322,14 @@
"@typescript-eslint/eslint-plugin": "^3.10.0",
"@typescript-eslint/parser": "^3.10.0",
"angular-aria": "^1.8.0",
"angular-mocks": "^1.7.9",
"angular-mocks": "^1.8.2",
"angular-recursion": "^1.0.5",
"angular-route": "^1.8.0",
"angular-sortable-view": "^0.0.17",
"archiver": "^3.1.1",
"axe-core": "^4.0.2",
"babel-eslint": "^10.0.3",
"babel-jest": "^26.3.0",
"babel-plugin-istanbul": "^6.0.0",
"babel-jest": "^27.5.1",
"brace": "0.11.1",
"chai": "3.5.0",
"chance": "1.0.18",
Expand All @@ -350,8 +350,8 @@
"eslint-plugin-ban": "^1.4.0",
"eslint-plugin-cypress": "^2.8.1",
"eslint-plugin-eslint-comments": "^3.2.0",
"eslint-plugin-import": "^2.19.1",
"eslint-plugin-jest": "^24.0.2",
"eslint-plugin-import": "^2.25.4",
"eslint-plugin-jest": "^26.1.1",
"eslint-plugin-jsx-a11y": "^6.2.3",
"eslint-plugin-mocha": "^6.2.2",
"eslint-plugin-no-unsanitized": "^3.0.2",
Expand All @@ -378,9 +378,8 @@
"history": "^4.9.0",
"immer": "^9.0.6",
"intl-messageformat-parser": "^1.4.0",
"jest": "^26.4.2",
"jest-canvas-mock": "^2.2.0",
"jest-environment-jsdom-thirteen": "^1.0.1",
"jest": "^27.5.1",
"jest-canvas-mock": "^2.3.1",
"jest-raw-loader": "^1.0.1",
"jimp": "^0.14.0",
"jquery": "^3.5.0",
Expand All @@ -406,7 +405,7 @@
"ngreact": "^0.5.1",
"nock": "12.0.3",
"normalize-path": "^3.0.0",
"nyc": "^14.1.1",
"nyc": "^15.1.0",
"pixelmatch": "^5.1.0",
"pngjs": "^3.4.0",
"postcss": "^8.4.5",
Expand All @@ -431,7 +430,7 @@
"simple-git": "1.116.0",
"sinon": "^7.4.2",
"strip-ansi": "^6.0.0",
"supertest": "^3.1.0",
"supertest": "^6.2.2",
"supertest-as-promised": "^4.0.2",
"tape": "^5.0.1",
"topojson-client": "3.0.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@
"eslint-plugin-ban": "^1.4.0",
"eslint-plugin-jsx-a11y": "^6.2.3",
"eslint-plugin-eslint-comments": "^3.2.0",
"eslint-plugin-import": "^2.19.1",
"eslint-plugin-jest": "^24.0.2",
"eslint-plugin-import": "^2.25.4",
"eslint-plugin-jest": "^26.1.1",
"eslint-plugin-mocha": "^6.2.2",
"eslint-plugin-no-unsanitized": "^3.0.2",
"eslint-plugin-prefer-object-spread": "^1.2.1",
Expand Down
2 changes: 1 addition & 1 deletion packages/osd-optimizer/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
"del": "^5.1.0",
"execa": "^4.0.2",
"file-loader": "^4.2.0",
"jest-diff": "^26.4.2",
"jest-diff": "^27.5.1",
"js-yaml": "^3.14.0",
"json-stable-stringify": "^1.0.1",
"lmdb-store": "^1.6.11",
Expand Down
6 changes: 3 additions & 3 deletions packages/osd-optimizer/src/optimizer/cache_keys.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@

import Path from 'path';

import jestDiff from 'jest-diff';
import { diff } from 'jest-diff';
import { REPO_ROOT } from '@osd/utils';
import { createAbsolutePathSerializer } from '@osd/dev-utils';

Expand Down Expand Up @@ -187,7 +187,7 @@ describe('diffCacheKey()', () => {

describe('reformatJestDiff()', () => {
it('reformats large jestDiff output to focus on the changed lines', () => {
const diff = jestDiff(
const jestDiff = diff(
{
a: ['1', '1', '1', '1', '1', '1', '1', '2', '1', '1', '1', '1', '1', '1', '1', '1', '1'],
},
Expand All @@ -196,7 +196,7 @@ describe('reformatJestDiff()', () => {
}
);

expect(reformatJestDiff(diff)).toMatchInlineSnapshot(`
expect(reformatJestDiff(jestDiff)).toMatchInlineSnapshot(`
"- Expected
+ Received
Expand Down
8 changes: 4 additions & 4 deletions packages/osd-optimizer/src/optimizer/cache_keys.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ import execa from 'execa';
import { REPO_ROOT } from '@osd/utils';
import stripAnsi from 'strip-ansi';

import jestDiff from 'jest-diff';
import { diff } from 'jest-diff';
import jsonStable from 'json-stable-stringify';
import { ascending, CacheableWorkerConfig } from '../common';

Expand All @@ -62,11 +62,11 @@ export function diffCacheKey(expected?: unknown, actual?: unknown) {
return;
}

return reformatJestDiff(jestDiff(expectedJson, actualJson));
return reformatJestDiff(diff(expectedJson, actualJson));
}

export function reformatJestDiff(diff: string | null) {
const diffLines = diff?.split('\n') || [];
export function reformatJestDiff(jestDiff: string | null) {
const diffLines = jestDiff?.split('\n') || [];

if (
diffLines.length < 4 ||
Expand Down
Loading

0 comments on commit 8da108a

Please sign in to comment.