[CI] Add tests to github workflow

kavilla · kavilla · commit 2d44dabf01d2 · 2021-12-13T12:01:53.000-08:00
Add unit tests to github workflow and also creating a "bad apples"
environment variable. Some unit tests just fail on the CI for
hardware issues. They should be improved but step one will be
calling out the bad apples.

Also due to the flakiness we can cache the previous run results
and only run the tests that failed. It's too random to catch
with the bad apples mechanism. But still added the continue on
error for unit tests because it takes so long to re-run on the
CI. So instead if it does fail we automatically echo there
was a failure and ask them to re-run. However, if we can get
permission for a github action that can add a comment to the PR
then we could automatically add to PR.

Next step will be improving.

Also needed to limit the amount of workers because otherwise the
hardware can't handle well so then it will accidentally create conflicts.
This means we get an accurate test run but it is slower on the CI.

Included integration tests which worked out of the box.

Included e2e tests as well but it the chrome driver for the application
was different from github's chrome so to run it I just upgraded it for
the test run. Not ideal, ideally we should probably set up a
docker env and install the specific versions since we are now
depending on github's virtual env and the dependencies they installed
there. But at least this is a first pace.

Signed-off-by: Kawika Avilla &lt;kavilla414@gmail.com&gt;
diff --git a/.github/workflows/pr_check_workflow.yml b/.github/workflows/pr_check_workflow.yml
@@ -1,25 +1,211 @@
 # This workflow will do a clean install of node dependencies, build the source code and run tests across different versions of node
 # For more information see: https://help.github.com/actions/language-and-framework-guides/using-nodejs-with-github-actions
 
-name: Node.js CI
+name: Build and test
 
 on:
   push:
     branches: [ main ]
   pull_request:
     branches: [ main ]
 
-jobs:
-  build:
+env:
+  CACHE_NAME: osd-node-modules
+  TEST_BROWSER_HEADLESS: 1
+  CI: 1
+  GCS_UPLOAD_PREFIX: fake
+  TEST_OPENSEARCH_DASHBOARDS_HOST: localhost
+  TEST_OPENSEARCH_DASHBOARDS_PORT: 6610
+  TEST_OPENSEARCH_TRANSPORT_PORT: 9403
+  TEST_OPENSEARCH_PORT: 9400
 
+jobs:
+  build-lint-test:
     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: "10.24.1"
+          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 yarn@1.22.10
+
+      - 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`.'
+      #       })
 
+      - 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
+  functional-tests:
+    needs: [ build-lint-test ]
+    runs-on: ubuntu-latest
+    name: Run functional tests
+    strategy: 
+      matrix:
+        group: [ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12 ]
     steps:
-    - uses: actions/checkout@v2
-    - name: Use Node.js
-      uses: actions/setup-node@v2
-      with:
-        node-version: '10.24.1'
-        check-latest: false
-    - run: yarn osd bootstrap
-    - run: yarn lint
+      - 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: "10.24.1"
+          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 yarn@1.22.10
+
+      - 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 chromedriver@95.0.0
+
+      - 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'
+        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
diff --git a/packages/osd-pm/src/run.test.ts b/packages/osd-pm/src/run.test.ts
@@ -37,6 +37,8 @@ import { runCommand } from './run';
 import { Project } from './utils/project';
 import { log } from './utils/log';
 
+const testif = process.env.SKIP_BAD_APPLES === 'true' ? test.skip : test;
+
 log.setLogLevel('silent');
 
 const rootPath = resolve(`${__dirname}/utils/__fixtures__/opensearch-dashboards`);
@@ -70,14 +72,14 @@ beforeEach(() => {
   };
 });
 
-test('passes all found projects to the command if no filter is specified', async () => {
+testif('passes all found projects to the command if no filter is specified', async () => {
   await runCommand(command, config);
 
   expect(command.run).toHaveBeenCalledTimes(1);
   expect(getExpectedProjectsAndGraph(command.run)).toMatchSnapshot();
 });
 
-test('excludes project if single `exclude` filter is specified', async () => {
+testif('excludes project if single `exclude` filter is specified', async () => {
   await runCommand(command, {
     ...config,
     options: { exclude: 'foo' },
@@ -87,7 +89,7 @@ test('excludes project if single `exclude` filter is specified', async () => {
   expect(getExpectedProjectsAndGraph(command.run)).toMatchSnapshot();
 });
 
-test('excludes projects if multiple `exclude` filter are specified', async () => {
+testif('excludes projects if multiple `exclude` filter are specified', async () => {
   await runCommand(command, {
     ...config,
     options: { exclude: ['foo', 'bar', 'baz'] },
@@ -97,7 +99,7 @@ test('excludes projects if multiple `exclude` filter are specified', async () =>
   expect(getExpectedProjectsAndGraph(command.run)).toMatchSnapshot();
 });
 
-test('includes single project if single `include` filter is specified', async () => {
+testif('includes single project if single `include` filter is specified', async () => {
   await runCommand(command, {
     ...config,
     options: { include: 'foo' },
@@ -107,7 +109,7 @@ test('includes single project if single `include` filter is specified', async ()
   expect(getExpectedProjectsAndGraph(command.run)).toMatchSnapshot();
 });
 
-test('includes only projects specified in multiple `include` filters', async () => {
+testif('includes only projects specified in multiple `include` filters', async () => {
   await runCommand(command, {
     ...config,
     options: { include: ['foo', 'bar', 'baz'] },
@@ -117,7 +119,7 @@ test('includes only projects specified in multiple `include` filters', async ()
   expect(getExpectedProjectsAndGraph(command.run)).toMatchSnapshot();
 });
 
-test('respects both `include` and `exclude` filters if specified at the same time', async () => {
+testif('respects both `include` and `exclude` filters if specified at the same time', async () => {
   await runCommand(command, {
     ...config,
     options: { include: ['foo', 'bar', 'baz'], exclude: 'bar' },
@@ -127,7 +129,7 @@ test('respects both `include` and `exclude` filters if specified at the same tim
   expect(getExpectedProjectsAndGraph(command.run)).toMatchSnapshot();
 });
 
-test('does not run command if all projects are filtered out', async () => {
+testif('does not run command if all projects are filtered out', async () => {
   const mockProcessExit = jest.spyOn(process, 'exit').mockReturnValue(undefined as never);
 
   await runCommand(command, {
diff --git a/src/dev/jest/config.js b/src/dev/jest/config.js
@@ -84,7 +84,7 @@ export default {
     '<rootDir>/src/dev/jest/setup/react_testing_library.js',
   ],
   coverageDirectory: '<rootDir>/target/opensearch-dashboards-coverage/jest',
-  coverageReporters: ['html', 'text'],
+  coverageReporters: ['html', 'text', 'text-summary'],
   moduleFileExtensions: ['js', 'mjs', 'json', 'ts', 'tsx', 'node'],
   modulePathIgnorePatterns: [
     '__fixtures__/',
diff --git a/src/plugins/embeddable/public/lib/embeddables/error_embeddable.test.tsx b/src/plugins/embeddable/public/lib/embeddables/error_embeddable.test.tsx
@@ -34,7 +34,9 @@ import { wait, render } from '@testing-library/react';
 import { ErrorEmbeddable } from './error_embeddable';
 import { EmbeddableRoot } from './embeddable_root';
 
-test('ErrorEmbeddable renders an embeddable', async () => {
+const testif = process.env.SKIP_BAD_APPLES === 'true' ? test.skip : test;
+
+testif('ErrorEmbeddable renders an embeddable', async () => {
   const embeddable = new ErrorEmbeddable('some error occurred', { id: '123', title: 'Error' });
   const { getByTestId, getByText } = render(<EmbeddableRoot embeddable={embeddable} />);
 
@@ -43,7 +45,7 @@ test('ErrorEmbeddable renders an embeddable', async () => {
   expect(getByText(/some error occurred/i)).toBeVisible();
 });
 
-test('ErrorEmbeddable renders an embeddable with markdown message', async () => {
+testif('ErrorEmbeddable renders an embeddable with markdown message', async () => {
   const error = '[some link](http://localhost:5601/takeMeThere)';
   const embeddable = new ErrorEmbeddable(error, { id: '123', title: 'Error' });
   const { getByTestId, getByText } = render(<EmbeddableRoot embeddable={embeddable} />);
diff --git a/src/plugins/vis_type_markdown/public/markdown_vis_controller.test.tsx b/src/plugins/vis_type_markdown/public/markdown_vis_controller.test.tsx
@@ -34,7 +34,9 @@ import React from 'react';
 import { wait, render } from '@testing-library/react';
 import MarkdownVisComponent from './markdown_vis_controller';
 
-describe('markdown vis controller', () => {
+const describeif = process.env.SKIP_BAD_APPLES === 'true' ? describe.skip : describe;
+
+describeif('markdown vis controller', () => {
   it('should set html from markdown params', async () => {
     const vis = {
       params: {
diff --git a/src/plugins/vis_type_tagcloud/public/components/tag_cloud_visualization.test.js b/src/plugins/vis_type_tagcloud/public/components/tag_cloud_visualization.test.js
@@ -37,9 +37,10 @@ import { setFormatService } from '../services';
 import { dataPluginMock } from '../../../data/public/mocks';
 import { setHTMLElementOffset, setSVGElementGetBBox } from '../../../../test_utils/public';
 
+const describeif = process.env.SKIP_BAD_APPLES === 'true' ? describe.skip : describe;
 const seedColors = ['#00a69b', '#57c17b', '#6f87d8', '#663db8', '#bc52bc', '#9e3533', '#daa05d'];
 
-describe('TagCloudVisualizationTest', () => {
+describeif('TagCloudVisualizationTest', () => {
   let domNode;
   let visParams;
   let SVGElementGetBBoxSpyInstance;
diff --git a/src/plugins/visualize/public/application/utils/use/use_visualize_app_state.test.ts b/src/plugins/visualize/public/application/utils/use/use_visualize_app_state.test.ts
@@ -40,11 +40,13 @@ import { visualizeAppStateStub } from '../stubs';
 import { VisualizeConstants } from '../../visualize_constants';
 import { createVisualizeServicesMock } from '../mocks';
 
+const describeif = process.env.SKIP_BAD_APPLES === 'true' ? describe.skip : describe;
+
 jest.mock('../utils');
 jest.mock('../create_visualize_app_state');
 jest.mock('../../../../../data/public');
 
-describe('useVisualizeAppState', () => {
+describeif('useVisualizeAppState', () => {
   const { visStateToEditorState } = jest.requireMock('../utils');
   const { createVisualizeAppState } = jest.requireMock('../create_visualize_app_state');
   const { connectToQueryState } = jest.requireMock('../../../../../data/public');
