kiwix / kiwix-js

Fully portable & lightweight ZIM reader in Javascript
https://www.kiwix.org/
GNU General Public License v3.0
300 stars 124 forks source link

Update QUnit to latest #653

Closed Jaifroid closed 3 years ago

Jaifroid commented 3 years ago

Latest QUnit is 2.22.2 from 09/09/2020.

(Maintaining dependencies would be much easier if we could start #554...)

rohitgeddam commented 3 years ago

I would like to work on this

Jaifroid commented 3 years ago

OK, thanks @rohitgeddam .

ghost commented 3 years ago

I would like to work on this

Jaifroid commented 3 years ago

@SakshiSarna OK!

ram690514 commented 3 years ago

I would like to work on this , but currently the latest version is 2.11.3

Jaifroid commented 3 years ago

Yes you're right @ram690514 . I don't know where I got that other number from. I see the previous person who offered has deleted their account, so I'll assign you.

ram690514 commented 3 years ago

thankyou @Jaifroid

ram690514 commented 3 years ago

i have created a pull request but it is showing some errors can you help me @Jaifroid

Jaifroid commented 3 years ago

@ram690514 Because you do not have write access to this repo, the tests won't run properly and will appear to fail. I can fix that -- but I notice you've closed your PR. Any reason for that?

ram690514 commented 3 years ago

@Jaifroid , i reopened it

ram690514 commented 3 years ago

@Jaifroid : I don't know but the latest version of Qunit is not working with this project , maybe it is because of my fault . can you please help me to fix this issue

Jaifroid commented 3 years ago

@ram690514 It would be helpful to know what you've tried. What errors are showing in dev console? You need a bit more informaiton than "it is not working" so that I can meaningfully help you!

ram690514 commented 3 years ago

I changed Qunit 2.3.2.js to Qunit2.11.3.js and also CSS file get replaced . but now when i load the test.html file it get show " 0 out of 4 files checked ". actually I updated by using npm command " npm install Qunit". I think may be there exist some other ways to update the existing Qunit file , may be you can help me. But at the same time i also found that there is no package.json file in this project . can you please explain a step by step method to fix this issue...please.

On Tue, Oct 20, 2020 at 9:06 PM Jaifroid notifications@github.com wrote:

@ram690514 https://github.com/ram690514 It would be helpful to know what you've tried. What errors are showing in dev console? You need a bit more informaiton than "it is not working" so that I can meaningfully help you!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/kiwix/kiwix-js/issues/653#issuecomment-712941208, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARAWVBVCD2P3HCCNJMCUFVTSLWU6JANCNFSM4R5OHJZQ .

Jaifroid commented 3 years ago

@ram690514 This is not an npm-enabled project. We have #554 for that issue. For now you have to update dependencies manually. If I were you I'd start again with master: make sure you know how to run the tests under the master branch and make sure they work. Then manually update just the QUnit files (by downloading and dropping in). You may need to look carefully at what has changed in the QUnit API, to be sure that the way in which tests.html and associated files are evoking it is still the correct one. I can't provide you with a step-by-step method, because then I'd be fixing this issue myself!

Youi have to go through patiently, one step at a time, and debug at each step. And don't use npm with this project, as it won't work.

mossroy commented 3 years ago

You were on the right track with #660 : it was the right way to upgrade QUnit, but some APIs have probably changed. Run tests.html to see what's wrong

Krinkle commented 3 years ago

I'd be happy to help update QUnit and address (some of) https://github.com/kiwix/kiwix-js/issues/554. I don't want to stand in the way if someone else is already working on this, however. Let me know!

Strawman proposal (feel free to suggest otherwise):

It should also be fairly straight-forward after this to plug in karma-coverage to produce a test coverage report (example 1, example 2) - ref https://github.com/kiwix/kiwix-js/issues/528.

Jaifroid commented 3 years ago

@Krinkle This would be very helpful. There was some work in #660 which seems to have stalled (and it was closed). So please do feel free to offer a PR. Note that we don't currently have an npm config for this project, but starting one might be a good way of easing in to #554 .

Jaifroid commented 3 years ago

PS I have no experience of Karma Runner, maybe @mossroy should comment on that part.

mossroy commented 3 years ago

@Krinkle : this contribution would be very welcome, and goes way further than this issue. We have no more news from the developer who created #660 since one month, and he removed himself from the assignees of this issue. So I suppose no one will be frustrated if you do it.

Let me give some more general background : You probably saw that we have 2 types of automatic tests run by the CI : unit tests with QUnit and end-to-end tests with nightwatch+saucelabs. This has been set up many years ago, and still works. But it deserves to be dusted off and nobody had time to do it. For example with #590 (we did not start it)

I've heard and used a bit Karma, but I don't have much experience on frontend testing frameworks. So I can't say which ones best suits our needs. As you seem to have this experience, if you think it's a good choice, I'm following you. In any case, I'm really interested in the pros you listed. And being able to have code coverage (#528) would also be a big plus!

NB : until #590 is done, the PRs made from forks of this repo can not run the CI. So, if you work on enhancing the CI, we'll need to give you more rights (probably add you as a member of kiwix team on github). @kelson42 : are you ok with that?