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

[CI] Add tests to github workflow #906

Closed
wants to merge 1 commit into from

Conversation

kavilla
Copy link
Member

@kavilla kavilla commented Nov 4, 2021

Description

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. 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 kavilla414@gmail.com

Issues Resolved

n/a

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff

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. 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 <kavilla414@gmail.com>
- name: Run unit tests
run: node scripts/jest --ci --colors --maxWorkers=10
env:
SKIP_BAD_APPLES: true
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean we will skip some unit tests below for now? Any plan to bring them back?

Copy link
Member Author

Choose a reason for hiding this comment

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

only for github actions. locally if this env var is not set it will still run them. the next step would be to 'quarantine' the bad apples and fix them or drop them. more likely to fix them since they work well on good hardware but work bad on bad hardware.

@kavilla kavilla added the ci label Nov 16, 2021
Copy link
Contributor

@tmarkley tmarkley left a comment

Choose a reason for hiding this comment

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

Awesome!

uses: actions/checkout@v2

- name: Setup Node
uses: actions/setup-node@v1
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a particular reason why we're not using v2?


# github virtual env is the latest chrome
- name: Setup chromedriver
run: yarn add --dev chromedriver@latest
Copy link
Contributor

Choose a reason for hiding this comment

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

To clarify, GitHub can only pull the latest? There's no way to specify a particular version?

Comment on lines 6 to 10
on:
push:
branches: [ main ]
pull_request:
branches: [ main ]
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we have this run on all branches? That would catch any potential unexpected merge issues/conflicts.

@kavilla
Copy link
Member Author

kavilla commented Dec 12, 2021

Preferring: #986 over this one for re-run purposes.

@kavilla kavilla closed this Dec 12, 2021
@kavilla kavilla deleted the avillk/github_wf branch April 12, 2022 06:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants