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

chore: remove karma + jasmine and replace with jest globally #4546

Merged
merged 21 commits into from
Oct 24, 2022

Conversation

aweiss-dev
Copy link
Member

@aweiss-dev aweiss-dev commented Oct 21, 2022

The goal of this PR is to streamline our Testrunner for every package in the Monorepo.

Until now we used a couple of solutions like:

  • jasmine
  • kasmine + karma
  • jest + babel
  • jest + ts-jest

The goal of this PR is:

  1. Use Jest for every package of the Monorepo
  2. Use Typescript for all of our test files
  3. Speed up test execution.
  4. Have a common config in the workspace root and just additions in the packages themselves
  5. Make life easier for future updates (Compiler changes etc.)

What has been done to archive the Goal:

  • Remove jasmine + karma globally
  • Transfer .js tests to .ts
  • Create a common jest config that is shared between all packages
  • Set up jest for every package
  • Make jest executable from root and the packed themselves

Open problems

1. Running jest from root

When running jest from root by executing "yarn jest", some package tests will fail because dependencies like TextEncoder can't be found or some global variables are not defined. I don't have a fix for that (yet), but its also not on the priority list since we execute scripts directly from the package folder (that don't have this problem).
As soon as we have a fix for this behaviour we can just execute jest from root to collect combined coverage of all packages. Also enables tools like Wallaby to be fully usable from root without having some tests error.

2. Running build before test execution

Some test (not many) need the built binaries from other packages to be able to execute their tests. Currently we build every package before running the tests, which costs a lot of time. We have a high potential to speed up the process if we cut down the build step before test execution.

@aweiss-dev aweiss-dev force-pushed the chore/remove_karma_jasmine branch from e4a84b9 to e0ebcc7 Compare October 21, 2022 15:55
@aweiss-dev aweiss-dev changed the title chore: remove karma + jasmine and replace with ts-jest globally chore: remove karma + jasmine and replace with jest globally Oct 21, 2022
Copy link
Contributor

@atomrc atomrc left a comment

Choose a reason for hiding this comment

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

Maybe we could try to hoist the import of jest and swc to the root package.json and remove them from every single package, WDYT?

packages/eslint-config/package.json Outdated Show resolved Hide resolved
packages/license-collector/package.json Outdated Show resolved Hide resolved
packages/prettier-config/package.json Outdated Show resolved Hide resolved
packages/webapp-events/package.json Outdated Show resolved Hide resolved
.prettierrc Outdated Show resolved Hide resolved
bin/testUpdated.js Outdated Show resolved Hide resolved
jest.config.js Outdated Show resolved Hide resolved
@aweiss-dev
Copy link
Member Author

Maybe we could try to hoist the import of jest and swc to the root package.json and remove them from every single package, WDYT?

I tried this. I could only run tests from root in this case and trying to run them from packages error Ed because the dependencies haven't been found. I think this is the case because the yarn.yaml hoists to packages per setting.

@atomrc
Copy link
Contributor

atomrc commented Oct 21, 2022

I tried this. I could only run tests from root in this case and trying to run them from packages error Ed because the dependencies haven't been found. I think this is the case because the yarn.yaml hoists to packages per setting.

I think it's because it's a binary that is executed and in this case it should be defined at the package level.
Let's try with swc dependencies, they might very well work (but for jest probably not indeed, was worth trying)

@aweiss-dev
Copy link
Member Author

I tried this. I could only run tests from root in this case and trying to run them from packages error Ed because the dependencies haven't been found. I think this is the case because the yarn.yaml hoists to packages per setting.

I think it's because it's a binary that is executed and in this case it should be defined at the package level. Let's try with swc dependencies, they might very well work (but for jest probably not indeed, was worth trying)

I actually found a solution. Enjoy :)

packages/certificate-check/package.json Show resolved Hide resolved
packages/cli-client/package.json Outdated Show resolved Hide resolved
packages/commons/package.json Outdated Show resolved Hide resolved
packages/priority-queue/package.json Show resolved Hide resolved
packages/priority-queue/package.json Outdated Show resolved Hide resolved
packages/promise-queue/package.json Show resolved Hide resolved
packages/react-ui-kit/package.json Show resolved Hide resolved
packages/store-engine/package.json Show resolved Hide resolved
Copy link
Contributor

@atomrc atomrc left a comment

Choose a reason for hiding this comment

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

Solid 💪

@aweiss-dev aweiss-dev merged commit 431df69 into main Oct 24, 2022
@aweiss-dev aweiss-dev deleted the chore/remove_karma_jasmine branch October 24, 2022 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants