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

Convert QUnit tests to mocha #1187

Open
Jaifroid opened this issue Jan 1, 2024 · 4 comments
Open

Convert QUnit tests to mocha #1187

Jaifroid opened this issue Jan 1, 2024 · 4 comments
Labels
build Code relating to building, publishing, or maintaining the app task tests
Milestone

Comments

@Jaifroid
Copy link
Member

Jaifroid commented Jan 1, 2024

Since we already use mocha for assertion in e2e tests, it would make sense to use it in Unit tests also. We currently run Unit tests in UI-based framework (QUnit) that runs directly in browsers, and as a result we have to run them in a browser-testing suite (testcafe) instead of directly in a Node framework like mocha. This adds overhead and is counterintuitive. It is also confusing, because there seems to be some blurring of lines between Unit and e2e testing.

I think it's time to drop QUnit.

@Jaifroid Jaifroid added tests build Code relating to building, publishing, or maintaining the app task labels Jan 1, 2024
@Jaifroid Jaifroid added this to the v4.2 milestone Jan 1, 2024
@gaurava05
Copy link

Hey, @Jaifroid. Would you like me to take this up? If yes, let me know if you have any existing ideas for this migration.

BTW, I got busy with work earlier, so couldn't continue on my previous issue, would like to work on something now.

@Jaifroid
Copy link
Member Author

@gaurava05 That would be great!

I don't have any preconceived ideas for the migration. We currently run Unit tests in a sort of quirky way, in browser context (for legacy reasons). They really just need to run in Node context, ideally without QUnit and using an industry-standard framework instead (I suggest Mocha, because it's already used in the Repo for Selenium-based e2e tests).

The idea is to ditch QUnit and Testcafé (the automation framework that only QUnit tests are run in) entirely. It will mean rewriting each test. Currently, QUnit tests are run without compiling the code, i.e. they are run directly on the raw codebase. I'm agnostic about whether it's better to run them after compiling to dist (npm run build-src or npm run build-min). I'm not sure what industry practice is here. In principle, as we're just testing small units of code, it probably is fine to continue testing uncompiled code.

You can see the existing code in /tests/unit/, and in particular the tests that will need rewriting for Node / Mocha are in /tests/unit/spec/tests.js. Best just to set up a simple test case with one test to ensure it is working before trying to migrate everything.

E2E tests are (of course) run on compiled code (using browser automation). But this issue wouldn't need to touch that testing code, as it's currently (more or less) stable and running well on Browser Stack.

@rishabhdwivedii
Copy link

Hey @Jaifroid,
I am trying to migrate this to mocha but facing some issues in mocking the image object,

> mocha ./tests/unit/spec/tests.js

ReferenceError: Image is not defined
    at file:///home/rishabh/gsoc/kiwix-js/tests/unit/js/init.js:40:16
    at file:///home/rishabh/gsoc/kiwix-js/tests/unit/js/init.js:45:3
    at ModuleJob.run (node:internal/modules/esm/module_job:218:25)
    at async ModuleLoader.import (node:internal/modules/esm/loader:329:24)
    at async importModuleDynamicallyWrapper (node:internal/vm/module:431:15)
    at async formattedImport (/home/rishabh/gsoc/kiwix-js/node_modules/mocha/lib/nodejs/esm-utils.js:9:14)
    at async exports.requireOrImport (/home/rishabh/gsoc/kiwix-js/node_modules/mocha/lib/nodejs/esm-utils.js:42:28)
    at async exports.loadFilesAsync (/home/rishabh/gsoc/kiwix-js/node_modules/mocha/lib/nodejs/esm-utils.js:100:20)
    at async singleRun (/home/rishabh/gsoc/kiwix-js/node_modules/mocha/lib/cli/run-helpers.js:125:3)
    at async exports.handler (/home/rishabh/gsoc/kiwix-js/node_modules/mocha/lib/cli/run.js:370:5)

Here's the link to the fork in which I've made the changes: https://github.com/rishabhdwivedii/kiwix-js

The changes I've made so far:

  1. Migrated the QUnit tests to mocha (but not changed the comments.
  2. Installed chai as dependency to use assert.
  3. To run the mocha test, added the mocha-test script in package.json.
  4. Added mocha to .eslintrc.cjs.

Can you guide me on how to deal with the Image reference error.

@Jaifroid
Copy link
Member Author

@rishabhdwivedii Thanks for having a go at this. You seem to be attempting to access the scripts over the file:// protocol, which won't work. You need to spin up a local server like http-server (included in the Repo if you did npm install). I'm afraid I don't know exactly how mocha works (testcafe includes its own server, or at least its test scripts start a server for you), so you'll need to check exactly what's needed to run tests in mocha.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Code relating to building, publishing, or maintaining the app task tests
Projects
None yet
Development

No branches or pull requests

3 participants