Skip to content
This repository has been archived by the owner on Apr 17, 2023. It is now read-only.

Refactor any mocha/ava tests to use Jest #192

Closed
3 tasks done
darahayes opened this issue Oct 10, 2019 · 9 comments
Closed
3 tasks done

Refactor any mocha/ava tests to use Jest #192

darahayes opened this issue Oct 10, 2019 · 9 comments

Comments

@darahayes
Copy link
Contributor

darahayes commented Oct 10, 2019

Background

Each package in /packages has a test command in package.json. Over time our team tried different testing frameworks in different packages but now we are starting to standardise around using Jest.

It would be really beneficial if we could use the one test runner across all packages. For anybody interested in picking up this issue I suggest that the tasks below are worked on in separate Pull Requests.

@dewstend
Copy link
Contributor

Hi! I would like to try my hand at these.

@wtrocki
Copy link
Contributor

wtrocki commented Oct 10, 2019

@dewstend Offix server was migrated already but if you have alternative PR feel free to create it.

@darahayes
Copy link
Contributor Author

Hey @dewstend the first two items in the list have been done but there's still one more if you're interested. We're here to help if you have any questions!

@dewstend
Copy link
Contributor

I investigated a bit, since Jest runs in JSDOM, it is not a trivial thing to run it in the browser, as briefly stated here.
There's the issue jestjs/jest#848, still open, for that matter.

I've seen a few community libraries that try to achieve Jest in headless, but nothing beyond obscure documentation.
Any suggestions?

@darahayes
Copy link
Contributor Author

@dewstend thanks for your investigation! So we've actually struggled a lot with those karma tests in the past. They're really hard to debug, the source maps support is kinda bad and the turnaround time to make edits and retest them are pretty bad.

Would you be interested in migrating those tests to use jest directly and we could simply remove karma? We already did some slight refactoring recently that should make this a bit easier to do.

I understand the downside here is that we wouldn't be testing in a real browser, but honestly the workflow and experience of using karma has been so poor that we probably would have written many more tests without it 😅

@dewstend
Copy link
Contributor

Is there documentation for the integration tests? I tried analyzing the code but it's quite sizeable, this migration seems like an issue on its own, also considering they are in JavaScript, maybe it would be preferable to standardize TypeScript and possibly using the same /test/ folder for both tests, and filename conventions for Jest pattern matching like so:

└───test
    ├───int
    │       File.int.test.ts
    │
    └───unit
            File.unit.test.ts

I investigated further and found out you the project runs Chrome Headless on Karma, Puppeteer (Headless Chrome Node.js API) can be implemented with Jest and there's documentation on it. This could be separated into tasks to achieve a full, better implementation of the current integration tests.

@wtrocki
Copy link
Contributor

wtrocki commented Oct 11, 2019

Technically there will be two elements that are browser specific:

  • NetworkState (Mocked in integration tests
  • Indexeddb (Also mocked)

So running tests on node should not be a problem at all. I would recommend to simply create PR that will change test runner to jest and then let team to resolve rest of the issues - if there are some

@darahayes
Copy link
Contributor Author

@dewstend thanks a lot for doing more investigation. @wtrocki made a strong point that in the current 'integration' tests, the NetworkState and IndexedDb interfaces are actually mocked. To us, running the tests in their current form in an actual browser environment adds little to no value. They are also harder to debug and they are written in JavaScript while the rest of the project is TypeScript.

I think ideally for now we would like to.

  • move all those tests over to the test folder to sit alongside the existing unit tests.
  • convert them to TypeScript and change the chai assertions to use Jest's expect.
  • have all tests being run by the test command.

I think there is still a lot value in creating a new issue to discuss/investigate using Jest + Puppeteer for a new set of integration tests that has very little mocking and covers some more advanced use cases. What do you folks think?

@wtrocki
Copy link
Contributor

wtrocki commented Oct 14, 2019

In proposed form this will be task for maintainer. I think we might just create couple smaller tasks for this with hacktoberfest label or do this ourselves

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants