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

Converted Q Unit tests to Mocha Tests #1301

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

THEBOSS0369
Copy link
Collaborator

@THEBOSS0369 THEBOSS0369 commented Jan 14, 2025

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

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

Screenshot for converted tests Passing

image

@THEBOSS0369
Copy link
Collaborator Author

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

@Jaifroid
Copy link
Member

@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 Jaifroid added this to the v4.2 milestone Jan 15, 2025
@Jaifroid Jaifroid added build Code relating to building, publishing, or maintaining the app task and removed enhancement labels Jan 15, 2025
@Jaifroid
Copy link
Member

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).

image

@Jaifroid
Copy link
Member

I haven't yet reviewed the code itself.

@THEBOSS0369
Copy link
Collaborator Author

@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
Copy link
Member

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
Copy link
Collaborator Author

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.

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

Successfully merging this pull request may close these issues.

Convert QUnit tests to mocha
2 participants