[CI] Unit tests are failing in GitHub Action for all PRs and Pushes

[tmarkley]

I just noticed that unit tests are not passing in the Build and Verify GitHub Action for any current PRs.

Example run for PR 1229 - see Run echo Unit tests completed unsuccessfully.

I went back through the commit history and it looks like unit tests have been failing for all commits since fc96cc3: Build and Verify unit tests failing

This might not be the cause, but the unit tests in each of the commits before this passed. The commit before that was 8777eb3: Build and Verify unit tests passing

[kavilla]

Just re-ran the PR for 1229, seems to be just the inconsistency issue since a new set of tests failed with the echo message being However, unit tests are inconsistent on the CI so please verify locally with yarn test:jest``.

From my understanding, the issue being github being limiting on the resources and then the tests run into the error
Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.Error:. So not to starve out the resources we limit the runners but it still it's flaky so we allowed it to pass without failing but for PRs I've been checking this step and just validating that any error generally tends to be just the jest timeout.

The snapshot error being an existing inconsistency. For Jenkins we end up updating snapshots every time we is probably not safe.

If the message needs to added to PR then will need give permissions in the org to an action that can add messages if the echo isn't helping.

To solve the flakiness, might just require not using github for PR validation or refactor the unit tests.

[tmarkley]

I think we need to re-consider how unit tests are run in GitHub Actions overall since it's taking them more than an hour to run (when successful) which is ridiculous considering they take less than a minute to run locally.

[tmarkley]

@kavilla also I don't think this is just flakiness since unit tests were consistently passing before that commit, based on going through commits before that.

All 5 of the commits were passing (on the first run) before that commit was pushed:

a8d499d: https://github.com/opensearch-project/OpenSearch-Dashboards/runs/4809365830?check_suite_focus=true
ddb2cc4: https://github.com/opensearch-project/OpenSearch-Dashboards/runs/4821234831?check_suite_focus=true
a765199: https://github.com/opensearch-project/OpenSearch-Dashboards/runs/4821425328?check_suite_focus=true
167b993: https://github.com/opensearch-project/OpenSearch-Dashboards/runs/4856193594?check_suite_focus=true
8777eb3: https://github.com/opensearch-project/OpenSearch-Dashboards/runs/4886751966?check_suite_focus=true

Could just be coincidence, but we need to figure out the root cause.

[kavilla]

https://github.com/opensearch-project/OpenSearch-Dashboards/runs/5070398658?check_suite_focus=true, is for a commit after fc96cc3 and succeeds on the first time.

[kavilla]

I think we need to re-consider how unit tests are run in GitHub Actions overall since it's taking them more than an hour to run (when successful) which is ridiculous considering they take less than a minute to run locally.

Yeah I agree it's really painful.

Right now in github we limit the amount of workers with maxWorkers=10:
https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/.github/workflows/pr_check_workflow.yml#L91

Locally when you run yarn test:jest it will just spawn a lot of workers, however without limiting the amount of workers in github it just ends still trying to spawn more workers but then github actions has a limited amount of resources it provides so then it just crashes (from my understanding).

[tmarkley]

https://github.com/opensearch-project/OpenSearch-Dashboards/runs/5070398658?check_suite_focus=true, is for a commit after fc96cc3 and succeeds on the first time.

Ah, nice catch. I must've missed that commit.

[tmarkley]

I reduced maxWorkers to 2 and unit tests passed in 11 minutes in my forked repo: https://github.com/tmarkley/OpenSearch-Dashboards/runs/5146756602?check_suite_focus=true

Going to try setting it to 1 to see if that improves it further. I've seen a few anecdotes like kulshekhar/ts-jest#1044 that suggest reducing maxWorkers to 1 or 2 to improve performance in GitHub actions.

[tmarkley]

I keep seeing this error:

PASS  src/plugins/data/server/search/routes/msearch.test.ts
(node:3072) UnhandledPromiseRejectionWarning: AbortError: Aborted
(Use `node --trace-warnings ...` to show where the warning was created)
Node.js process-warning detected:
UnhandledPromiseRejectionWarning: AbortError: Aborted
    at emitUnhandledRejectionWarning (internal/process/promises.js:170:15)
    at processPromiseRejections (internal/process/promises.js:247:11)
    at processTicksAndRejections (internal/process/task_queues.js:96:32)
Terminating process...
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
Error: Process completed with exit code 1.

Example: https://github.com/tmarkley/OpenSearch-Dashboards/runs/5160718614?check_suite_focus=true

[tmarkley]

Also trying to use --runInBand. Reference: https://dev.to/vantanev/make-your-jest-tests-up-to-20-faster-by-changing-a-single-setting-i36

[kavilla]

Also trying to use --runInBand. Reference: https://dev.to/vantanev/make-your-jest-tests-up-to-20-faster-by-changing-a-single-setting-i36

As far as am I aware runInBand would look similar to what maxWorkers=1 would be.

[kavilla]

I reduced maxWorkers to 2 and unit tests passed in 11 minutes in my forked repo: https://github.com/tmarkley/OpenSearch-Dashboards/runs/5146756602?check_suite_focus=true

This is interesting. So reducing the amount of workers that can spawn makes it goes faster? Seems kind of anti-intuitive. I believe I remember trying this before and it took nearly 4 hours. Perhaps part of the node upgrade relieved some of the issues?

If maxWorkers=2 is the secret sauce but I wonder if there are other values that we can try. Like 3, 4, 5. Does it improve and consistence if it works then I would totally be excited to get this! Seems like there are other articles that present it as configure it and find out what works best for us.

[kavilla]

Remove checksum flag check

[tmarkley]

Remove checksum flag check

@kavilla I tried removing the flag check and it still failed 😞