Converted Q Unit tests to Mocha Tests #1301

THEBOSS0369 · 2025-01-14T06:32:32Z

This PR Fixes #1187

Description

In this PR i have converted Q Unit tests to Mocha Tests.

Tests Converted

All the tests from unit/spec/test.js file are converted into mocha tests. It can be found in unit/spec/mocha.test.js

Note:
In this PR i haven't deleted the test.js file as once other files are approved we can continue with deleting that file

Commands to Run the test:

npm run test-unit : This command runs the unit tests for the application to ensure that individual units of code (such as functions or methods) are working as expected.

npm run test-unit-watch : This command allows us to continuously run tests while we work on our code. If we change a test file or the code it tests, Mocha will automatically re-run the tests.

npm run test-unit-coverage : This command runs the unit tests with code coverage reporting using NYC, which is a tool for generating code coverage reports.

Test

I have done all the necessary test

Checked in both "Restricted" and "ServiceWorker" Everything is working fine.
Unit tests npm test no issue
End-to-end (e2e) tests-e2e-firefox-> All tests Passed.
extension versions with production code tested
Browser Test -> Microsoft Edge.

Screenshot for converted tests Passing

THEBOSS0369 · 2025-01-14T06:42:09Z

These tests are falling because of the changes done in init.js file, I am fixing that right now

Signed-off-by: THEBOSS0369 <[email protected]>

…ix-js into con-mocha-backup

Jaifroid · 2025-01-15T07:22:21Z

@THEBOSS0369 Thank you very much for this, that's great! I'll take a look as soon as I can. Looks like a great improvement. @audiodude maybe you have some comments?

Jaifroid · 2025-01-18T12:53:21Z

I've done some basic tests by switching to the PR/branch, running npm install, and npm run test-unit and test-unit-coverage. Congratulations, this is very impressive conversion work!

Just quickly, I noticed that running these commands create a bunch of temporary files in the Repo that have not been correctly ignored (see screenshot). If the coverage files are overwritten on each coverage run, then they can remain, but you should .gitignore the coverage folder. Regarding the .nyc_output folder, it should also be .gitignore'd, but these look like files with a PID, and so I imagine will be created every run and could become a nuisance in that folder. Ignoring the folder may not be enough. Please check if there is a way to prevent them being generated, or whether they get auto-cleaned up (which would be fine).

Jaifroid · 2025-01-18T12:53:35Z

I haven't yet reviewed the code itself.

THEBOSS0369 · 2025-01-18T13:22:16Z

@Jaifroid Sir the temp files that you are seeing is not from the code instead it is due to the command npm run test-unit-coverage which is present in package.json file. The json files are the report files. The report gets updated when we re run the command and there is a change in files.
I didn't thought of that it would install the files in the main repo itself. However if you want i can delete the command so that these files wouldn't get downloaded and once this pr passes all the ticks we can add the command in the docs so if anyone wants report they can directly copy paste the command. Let me know what you think

This is a blog for a reference https://zinserjan.github.io/mocha-webpack/docs/guides/code-coverage.html

Jaifroid · 2025-01-18T14:29:16Z

Yes, I was aware that these reports were from running the command, but we don't want them being accidentally pushed to the server by a dev, so the folders containing them should at least be added to .gitignore. I don't think it's necessary to remove the command to invoke the code coverage test. I noticed the coverage hadn't been fully set up, it doesn't list any files and coverage currently.

THEBOSS0369 · 2025-01-18T16:34:15Z

Yes, I was aware that these reports were from running the command, but we don't want them being accidentally pushed to the server by a dev, so the folders containing them should at least be added to .gitignore. I don't think it's necessary to remove the command to invoke the code coverage test. I noticed the coverage hadn't been fully set up, it doesn't list any files and coverage currently.

Got it sir i will do it, and yes right now it just shows the table's heading and empty value I wanted to confirm whether this feature is needed or not so i created as an example. Once conversion work of test is approved, I will make it working.

THEBOSS0369 · 2025-02-03T06:31:03Z

Hey @Jaifroid sir let me know what to do further in this pr, when you get some time.

Jaifroid · 2025-02-03T07:25:14Z

Thanks! I noticed fallback errors in the test run due to Fetch not being supported in the test environment (see screenshot):

To avoid this, we should add a test for 'Fetch' support in the files xzdec_wrapper.js and zstddec_wrapper.js. It is sufficient to add this in each file where we test for WebAssembly support:

if ('WebAssembly' in self && 'Fetch' in self) { (the new code is && 'Fetch' in self).

Add this specifically on these lines:

Jaifroid

@THEBOSS0369 It's looking good. Just some minor suggestions to clean up the output display.

tests/unit/spec/mocha.test.js

Signed-off-by: THEBOSS0369 <[email protected]>

THEBOSS0369 · 2025-02-03T12:33:35Z

Thanks! I noticed fallback errors in the test run due to Fetch not being supported in the test environment (see screenshot):

To avoid this, we should add a test for 'Fetch' support in the files xzdec_wrapper.js and zstddec_wrapper.js. It is sufficient to add this in each file where we test for WebAssembly support:

if ('WebAssembly' in self && 'Fetch' in self) { (the new code is && 'Fetch' in self).

Add this specifically on these lines:

https://github.com/kiwix/kiwix-js/blob/main/www/js/lib/xzdec_wrapper.js#L34

https://github.com/kiwix/kiwix-js/blob/main/www/js/lib/zstddec_wrapper.js#L 98

@Jaifroid Thanks you for this sir , this was such a help i was trying to fix this in files zstddec_asm.js and wasm and similar types of files and was just about to ask you about this. I have fixed it now and other typos as you suggested. please take a look

THEBOSS0369 · 2025-02-05T07:29:23Z

Progress status: I'm currently trying to fix npm run test-unit-coverage isse. Its taking longer than i expected it to plus my winter's holiday has ended 2 weeks ago so i am not getting enough time to focus only on fixing it but i will try to fix it till this weekend, if i ain't able to make it, i will probably won't add the feature in this pr. would that be fine sir? @Jaifroid

Jaifroid · 2025-02-05T08:57:37Z

Progress status: I'm currently trying to fix npm run test-unit-coverage isse. Its taking longer than i expected it to plus my winter's holiday has ended 2 weeks ago so i am not getting enough time to focus only on fixing it but i will try to fix it till this weekend, if i ain't able to make it, i will probably won't add the feature in this pr. would that be fine sir? @Jaifroid

Thanks for the update. See what you can do this weekend, and if it's too complicated, maybe you could just make it into a separate issue to "Add Unit test coverage".

Signed-off-by: THEBOSS0369 <[email protected]>

THEBOSS0369 · 2025-02-08T11:51:21Z

Hello @Jaifroid sir i have fixed it but its creating so many problems in other workflow of the repo's.

First is that the main test are failing if i add the functionality it shows error message in files like urlutils.js , settingstore.js... and when i fix them it disturb the main workflow
second is the loaders which is used for testing will most probably be removed in future which will most probably disturb the workflow of repo, and when i use the instead option showing in below ss, it does not suite with other things.

example is this

coverage report are showing but with error messages

I am not very confident with this so i am just commiting the code to show you. I am just confirming again that i should remove this functionality from this pr and go ahead with the creating an issue or should i work on it more.

Its just keep getting complicated & more complicated and i don't want to mess with the codebase. Let me know what to do thanks.

THEBOSS0369 · 2025-02-08T11:53:46Z

I will revert the recent commit once you have taken a look

tests/unit/coverage-setup.cjs

.babelrc

Jaifroid · 2025-02-08T13:23:11Z

@THEBOSS0369 I didn't review the whole of the latest commit, but I immediately noticed you're using the wrong type of modules. We use ES6 Modules, with import statements, not CommonJS modules with require statements. If ever you are copying code from Stack Exchange that has require in it, then it won't work with our project. It took a lot of work to get rid of the old moduling system and modernize it to ES6!

Overall, my view is that you should revert the code coverage commit and make it into a separate issue. It's enough that you've converted our ancient tests to a modern format. We know the coverage is extremely poor currently, and we need to work on adding more tests for basic units of code. Being told on each run that our coverage is poor won't really help the situation, even though it's a good idea in principle! Many thanks!

This reverts commit 444e215.

THEBOSS0369 · 2025-02-08T13:57:10Z

@THEBOSS0369 I didn't review the whole of the latest commit, but I immediately noticed you're using the wrong type of modules. We use ES6 Modules, with import statements, not CommonJS modules with require statements. If ever you are copying code from Stack Exchange that has require in it, then it won't work with our project. It took a lot of work to get rid of the old moduling system and modernize it to ES6!

Overall, my view is that you should revert the code coverage commit and make it into a separate issue. It's enough that you've converted our ancient tests to a modern format. We know the coverage is extremely poor currently, and we need to work on adding more tests for basic units of code. Being told on each run that our coverage is poor won't really help the situation, even though it's a good idea in principle! Many thanks!

Got it sir i have reverted the changes and yess i have been searching out the internet to make it work from last 2 weeks as i have never worked on something like this before 😅🫠.
I think the work is done now so do i need to do anything else on this pr? if not then i will move to different issue.

Jaifroid · 2025-02-09T10:46:37Z

Thanks, @THEBOSS0369 🙏. I shall review ASAP.

THEBOSS0369 · 2025-03-01T14:36:47Z

Hello @Jaifroid Sir i have my exams going on and it will end on 10 march . so there won't be any problem if you can review this pr after 10th march. Thanks.

Jaifroid · 2025-03-16T17:16:26Z

@THEBOSS0369 I hope your exams went well! Let's get this PR finished and merged!

It strikes me that what remains to be done is to integrate the new tests into the workflows, and remove testcafe, as this won't be needed going forwards. There is also the old tests.js code to remove.

I notice that some security updates have introduced a conflict in your PR. You may be able to resolve this by running npm audit fix in the root of this repo for your PR. Then npm install, and push the changes to package-lock.json and package.json.

Jaifroid · 2025-03-16T17:18:01Z

Also, npm test should be changed to run the new tests locally rather than with testcafé.

Signed-off-by: THEBOSS0369 <[email protected]>

…-backup

Signed-off-by: THEBOSS0369 <[email protected]>

THEBOSS0369 · 2025-03-17T08:01:17Z

@Jaifroid Sir Thanks for that my exams went very well!

Now I have integrated the test in the workflows, deleted the test.js & inittestcafe.js file and removed the testcafe dependency . The tests are working fine in my local setup. Please review and let me know if anything else is required.

tests/unit/js/init.js

Jaifroid · 2025-03-17T10:18:28Z

It's looking great! I've tested manually in both Windows and WSL (Ubuntu), and no issues with npm test, npm run test-unit, or npm test-unit-coverage.

The one command causing issues is npm test-unit-watch. It produces this output in Windows terminal, and similar in Linux terminal:

PS C:\Repos\kiwix-js> npm run test-unit-watch

> [email protected] test-unit-watch
> mocha --experimental-modules --node-option experimental-specifier-resolution=node --watch --require @babel/register --require tests/unit/setup.js 'tests/unit/spec/**/*.test.js'

ReferenceError: require is not defined
    at file:///C:/Repos/kiwix-js/tests/unit/spec/mocha.test.js:4:1
    at ModuleJobSync.runSync (node:internal/modules/esm/module_job:395:35)
    at ModuleLoader.importSyncForRequire (node:internal/modules/esm/loader:329:47)
    at loadESMFromCJS (node:internal/modules/cjs/loader:1414:24)
    at Module._compile (node:internal/modules/cjs/loader:1547:5)
    at Module._compile (C:\Repos\kiwix-js\node_modules\pirates\lib\index.js:117:24)
    at node:internal/modules/cjs/loader:1677:16
    at Object.newLoader [as .js] (C:\Repos\kiwix-js\node_modules\pirates\lib\index.js:121:7)
    at Module.load (node:internal/modules/cjs/loader:1318:32)
    at Function._load (node:internal/modules/cjs/loader:1128:12)
    at TracingChannel.traceSync (node:diagnostics_channel:322:14)
    at wrapModuleLoad (node:internal/modules/cjs/loader:219:24)
    at Module.require (node:internal/modules/cjs/loader:1340:12)
    at require (node:internal/modules/helpers:138:16)
    at C:\Repos\kiwix-js\node_modules\mocha\lib\mocha.js:416:36
    at Array.forEach (<anonymous>)
    at Mocha.loadFiles (C:\Repos\kiwix-js\node_modules\mocha\lib\mocha.js:413:14)
    at Mocha.run (C:\Repos\kiwix-js\node_modules\mocha\lib\mocha.js:988:10)
    at Object.run (C:\Repos\kiwix-js\node_modules\mocha\lib\cli\watch-run.js:263:22)
    at FSWatcher.<anonymous> (C:\Repos\kiwix-js\node_modules\mocha\lib\cli\watch-run.js:184:14)
    at processTicksAndRejections (node:internal/process/task_queues:105:5)
file:///C:/Repos/kiwix-js/tests/unit/spec/mocha.test.js:4
require("core-js/modules/es.symbol.async-iterator.js");
^

It looks like a rogue require in dependencies, which probably won't work because we're using ESM modules. However, I can't find require in the file it mentions (mocha.test.js line 4).

Does the command test-unit-watch work for you?

Signed-off-by: THEBOSS0369 <[email protected]>

THEBOSS0369 · 2025-03-17T12:33:24Z

It looks like a rogue require in dependencies, which probably won't work because we're using ESM modules. However, I can't find require in the file it mentions (mocha.test.js line 4).

Does the command test-unit-watch work for you?

@Jaifroid sir i didn't noticed it earlier and yes its not working for me as well. Most probably the reason you stated is correct although it was working last month though am not sure what happened, i looked on google wasn't able to find anything other than Module compatible issue. So i am removing it as its not working as expected and our main command npm-test-unit shows the result in fraction of second so i don't think other developers may find any time issues while using it.

Also my log shows a little bit different result
here

npm run test-unit-watch

> kiwix-js@4.1.2 test-unit-watch
> mocha --experimental-modules --node-option experimental-specifier-resolution=node --watch --require @babel/register --require tests/unit/setup.js 'tests/unit/spec/**/*.test.js'

Error [ERR_REQUIRE_ESM]: require() of ES Module /home/boss/realcode/opnsrc/kiwix/kiwix-js/tests/unit/spec/mocha.test.js from /home/boss/realcode/opnsrc/kiwix/kiwix-js/node_modules/mocha/lib/mocha.js not supported.
Instead change the require of mocha.test.js in /home/boss/realcode/opnsrc/kiwix/kiwix-js/node_modules/mocha/lib/mocha.js to a dynamic import() which is available in all CommonJS modules.
    at Object.newLoader [as .js] (/home/boss/realcode/opnsrc/kiwix/kiwix-js/node_modules/pirates/lib/index.js:121:7)
    at /home/boss/realcode/opnsrc/kiwix/kiwix-js/node_modules/mocha/lib/mocha.js:416:36
    at Array.forEach (<anonymous>)
    at Mocha.loadFiles (/home/boss/realcode/opnsrc/kiwix/kiwix-js/node_modules/mocha/lib/mocha.js:413:14)
    at Mocha.run (/home/boss/realcode/opnsrc/kiwix/kiwix-js/node_modules/mocha/lib/mocha.js:988:10)
    at Object.run (/home/boss/realcode/opnsrc/kiwix/kiwix-js/node_modules/mocha/lib/cli/watch-run.js:263:22)
    at FSWatcher.<anonymous> (/home/boss/realcode/opnsrc/kiwix/kiwix-js/node_modules/mocha/lib/cli/watch-run.js:184:14)

Jaifroid · 2025-03-17T12:47:12Z

No problem about removing the command, it's probably not so useful for the reasons you state.

Jaifroid

Great, tests are passing, and I think this is ready now! Thank you very much for this great PR 🙏😊. Please feel free to squash and merge, or tell me if you don't have access to do that (I think you should have access).

THEBOSS0369 · 2025-03-17T12:56:33Z

Great, tests are passing, and I think this is ready now! Thank you very much for this great PR 🙏😊. Please feel free to squash and merge, or tell me if you don't have access to do that (I think you should have access).

I have the access sir and i am squashing and merging it now

THEBOSS0369 added 2 commits January 14, 2025 11:48

Added Files

eca978c

Merge branch 'main' into con-mocha-backup

d799c48

THEBOSS0369 added 3 commits January 14, 2025 13:10

Fixed the init file

e775e9c

Signed-off-by: THEBOSS0369 <[email protected]>

Merge branch 'con-mocha-backup' of https://github.com/THEBOSS0369/kiw…

6a0a5d9

…ix-js into con-mocha-backup

Trying again

8ccbdd5

Jaifroid assigned THEBOSS0369 Jan 15, 2025

Jaifroid added enhancement tests labels Jan 15, 2025

Jaifroid added this to the v4.2 milestone Jan 15, 2025

Jaifroid added build Code relating to building, publishing, or maintaining the app task and removed enhancement labels Jan 15, 2025

Merge branch 'main' into con-mocha-backup

3982fc8

Adding coverage folders in gitignore

7bcbe82

Jaifroid self-requested a review January 26, 2025 11:34

THEBOSS0369 added 2 commits January 26, 2025 17:07

Merge branch 'main' into con-mocha-backup

93b98a6

Merge branch 'main' into con-mocha-backup

01d35a6

Jaifroid requested changes Feb 3, 2025

View reviewed changes

Fixed fallback errors & some grammatical checks

2a5cb69

Signed-off-by: THEBOSS0369 <[email protected]>

Added unit test coverage

444e215

Signed-off-by: THEBOSS0369 <[email protected]>

Jaifroid reviewed Feb 8, 2025

View reviewed changes

tests/unit/coverage-setup.cjs Outdated Show resolved Hide resolved

Jaifroid reviewed Feb 8, 2025

View reviewed changes

.babelrc Outdated Show resolved Hide resolved

Revert "Added unit test coverage"

7cba4e3

This reverts commit 444e215.

THEBOSS0369 mentioned this pull request Feb 9, 2025

Add Unit test coverage #1308

Open

Merge branch 'main' into con-mocha-backup

822c0b1

THEBOSS0369 added 4 commits March 17, 2025 12:02

Merge branch 'main' into con-mocha-backup

c0ad905

Signed-off-by: THEBOSS0369 <[email protected]>

Merge remote-tracking branch 'origin/con-mocha-backup' into con-mocha…

9a41323

…-backup

Fixing merge conflicts

6c8888c

Signed-off-by: THEBOSS0369 <[email protected]>

Integrate tests into workflows

721b72b

Signed-off-by: THEBOSS0369 <[email protected]>

Jaifroid reviewed Mar 17, 2025

View reviewed changes

tests/unit/js/init.js Outdated Show resolved Hide resolved

removing unit-watch

bfc5aa4

Signed-off-by: THEBOSS0369 <[email protected]>

Jaifroid approved these changes Mar 17, 2025

View reviewed changes

THEBOSS0369 merged commit b3c4044 into kiwix:main Mar 17, 2025
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Converted Q Unit tests to Mocha Tests #1301

Converted Q Unit tests to Mocha Tests #1301

THEBOSS0369 commented Jan 14, 2025 •

edited

Loading

THEBOSS0369 commented Jan 14, 2025

Jaifroid commented Jan 15, 2025

Jaifroid commented Jan 18, 2025

Jaifroid commented Jan 18, 2025

THEBOSS0369 commented Jan 18, 2025

Jaifroid commented Jan 18, 2025

THEBOSS0369 commented Jan 18, 2025

THEBOSS0369 commented Feb 3, 2025

Jaifroid commented Feb 3, 2025

Jaifroid left a comment

THEBOSS0369 commented Feb 3, 2025

THEBOSS0369 commented Feb 5, 2025

Jaifroid commented Feb 5, 2025

THEBOSS0369 commented Feb 8, 2025

THEBOSS0369 commented Feb 8, 2025 •

edited

Loading

Jaifroid commented Feb 8, 2025 •

edited

Loading

THEBOSS0369 commented Feb 8, 2025

Jaifroid commented Feb 9, 2025

THEBOSS0369 commented Mar 1, 2025

Jaifroid commented Mar 16, 2025

Jaifroid commented Mar 16, 2025

THEBOSS0369 commented Mar 17, 2025 •

edited

Loading

Jaifroid commented Mar 17, 2025

THEBOSS0369 commented Mar 17, 2025 •

edited

Loading

Jaifroid commented Mar 17, 2025

Jaifroid left a comment

THEBOSS0369 commented Mar 17, 2025

Converted Q Unit tests to Mocha Tests #1301

Converted Q Unit tests to Mocha Tests #1301

Conversation

THEBOSS0369 commented Jan 14, 2025 • edited Loading

Description

Tests Converted

Test

Screenshot for converted tests Passing

THEBOSS0369 commented Jan 14, 2025

Jaifroid commented Jan 15, 2025

Jaifroid commented Jan 18, 2025

Jaifroid commented Jan 18, 2025

THEBOSS0369 commented Jan 18, 2025

Jaifroid commented Jan 18, 2025

THEBOSS0369 commented Jan 18, 2025

THEBOSS0369 commented Feb 3, 2025

Jaifroid commented Feb 3, 2025

Jaifroid left a comment

Choose a reason for hiding this comment

THEBOSS0369 commented Feb 3, 2025

THEBOSS0369 commented Feb 5, 2025

Jaifroid commented Feb 5, 2025

THEBOSS0369 commented Feb 8, 2025

THEBOSS0369 commented Feb 8, 2025 • edited Loading

Jaifroid commented Feb 8, 2025 • edited Loading

THEBOSS0369 commented Feb 8, 2025

Jaifroid commented Feb 9, 2025

THEBOSS0369 commented Mar 1, 2025

Jaifroid commented Mar 16, 2025

Jaifroid commented Mar 16, 2025

THEBOSS0369 commented Mar 17, 2025 • edited Loading

Jaifroid commented Mar 17, 2025

THEBOSS0369 commented Mar 17, 2025 • edited Loading

Jaifroid commented Mar 17, 2025

Jaifroid left a comment

Choose a reason for hiding this comment

THEBOSS0369 commented Mar 17, 2025

THEBOSS0369 commented Jan 14, 2025 •

edited

Loading

THEBOSS0369 commented Feb 8, 2025 •

edited

Loading

Jaifroid commented Feb 8, 2025 •

edited

Loading

THEBOSS0369 commented Mar 17, 2025 •

edited

Loading

THEBOSS0369 commented Mar 17, 2025 •

edited

Loading