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

fix: added Makefile target to run tests with Karma #1851

Merged
merged 1 commit into from
Dec 13, 2022

Conversation

MaxFrank13
Copy link
Member

This adds a new Makefile target that will install Firefox and run the tests using Karma. There is also a PR template that will remind future contributors to run these tests locally. Once a fix is found for getting these tests to run in CI, we will need to remove this PR template.

Copy link
Contributor

@justinhynes justinhynes left a comment

Choose a reason for hiding this comment

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

Good stuff! One requested change before accepting this change. Let me know if you have any questions!

Makefile Outdated
@@ -119,6 +119,10 @@ tests: ## Run tests and generate coverage report
js-tests: ## Run javascript tests
npm run test-react

test-karma: ## Run JS tests through Karma & install firefox. This command needs to be ran manually in the devstack container before submitting a pull request. It can not be run in CI as of APER-2136.
sudo apt-get install --no-install-recommends -y firefox
Copy link
Contributor

Choose a reason for hiding this comment

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

Just in case, I would also add a line before this to update the package information for all the configured sources.

So, before the sudo apt-get install... line, add sudo apt-get update

Copy link
Contributor

Choose a reason for hiding this comment

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

I think in the original CI workflow we were also installing xvfb. Maybe we should also include that package to cover our bases?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good call! The command now installs xvfb and then uses xvfb-run similar to how it's done here

@MaxFrank13 MaxFrank13 force-pushed the mfrank/karma_tests_make_target branch from 9da5faf to c9b0479 Compare December 7, 2022 18:41
@MaxFrank13 MaxFrank13 merged commit 8650181 into master Dec 13, 2022
@MaxFrank13 MaxFrank13 deleted the mfrank/karma_tests_make_target branch December 13, 2022 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants