Page MenuHomePhabricator

Investigate switching Javascript unit tests from QUnit to Jest [12hrs]
Closed, ResolvedPublicSpike

Description

After investigation and conversation, we're going to move to jest for new js component/module work - @jsn.sherman

Switching PageTriage's Javascript unit tests from QUnit to Jest was suggested by @jsn.sherman. I'd like to explore the idea more in this ticket.

Investigate

  • One of our goals should be to migrate PageTriage to some of the most common MediaWiki technologies in use. This lets more MediaWiki developers work on this repo, with less brainpower expended. I think QUnit is used much more than Jest in the MediaWiki world at the moment. So that's a good argument for using QUnit.
  • But I hear that Jest is particularly good for testing Vue?

Findings:

qunit is definitely widespread, but there is precedence for using jest in mediawiki. Jest is more straightfoward to actually run (locally or in ci) since all it require is npm install && npm run jest vs the more significant qunit setup.

Investigate:

  • There's only one JavaScript test file in the PageTriage repo at the moment, and it's written in QUnit, and is around 250 lines of code. If we change to Jest, should we bother rewriting this file, or should we just use both unit test frameworks side-by-side?
    • Jason's note on the suggestion to use both test frameworks, from Discord: "at the macro level those kinds of decisions are exactly how we ended up where we are today: multiple technologies used to do the same kind of job in different parts of the codebase. My baseline for stuff we are moving away from is to ditch it any time it's a light lift."

Findings:
Given the amount of untested code, everyone's feedback, and the fact that everybody wants to see the code covered by the existing tests replaced, I agree to just leaving the qunit tests in place - getting new coverage and working towards replacing the existing code is a reasonable plan. - @jsn.sherman

Investigate
How would jest be impacted by a future move to vitest T305515: Consider migrating from Jest to Vitest

Findings:
vitest offers a (mostly) jest compatible api and it has been designed to be easy to migrate from jest. It should be a straightforward migration, should it need to happen in the future.
https://vitest.dev/guide/migration.html

cc @kostajh

Event Timeline

One of our goals should be to migrate PageTriage to some of the most common MediaWiki technologies in use.

If that's the case, then vanilla vue.js itself could be ruled out, as ooui is far more commonly implemented. An argument could be made to move to codex (the new vue.js component library that implements the current design system standards), but that would make the most sense if we're allowed to change the styling of the components, since the whole point of the library is to follow the current ui standards which the current interface does not implement.

But I hear that Jest is particularly good for testing Vue?

From my perspective, the biggest advantages are:

  • its high rate of adoption outside of mediawiki (meaning more new people could look at those tests and understand them and follow guides to write new tests)
  • it's easy to setup with mock-mediawiki https://github.com/wikimedia-gadgets/mock-mediawiki
  • It's easy to get coverage reports
  • My belief that it's probably where the mediawiki ecosystem is also headed; there are a number of things in production using jest, including the aforementioned codex library. To be clear, this point is speculative on my part.

There's only one JavaScript test file in the PageTriage repo at the moment, and it's written in QUnit, and is around 250 lines of code. If we change to Jest, should we bother rewriting this file, or should we just use both unit test frameworks side-by-side?
Jason's note on the suggestion to use both test frameworks, from Discord:

"at the macro level those kinds of decisions are exactly how we ended up where we are today: multiple technologies used to do the same kind of job in different parts of the codebase. My baseline for stuff we are moving away from is to ditch it any time it's a light lift."

Yeah, having multiple unit test frameworks means that someone has to understand which framework covers which parts of the code when they want to add a new test, meaning they would ultimately need to learn both frameworks. Or, if the plan is to migrate a test over to jest any time they get touched, the person doing the change has to pay that context switching cost ad hoc for a small change. A good example is the fact that we still have jquery-ui code in production. Depending on what thing a developer is trying to improve, they may need to (re)familariaze themselves with a framework that we officially moved away from years ago.

I believe we should have exactly 1 javascript unit testing framework, and I believe that strongly enough that I'd rather stick with qunit and not implement jest rather than having both. I just happen to think that jest is a better fit for where this extension is headed.

I believe we should have exactly 1 javascript unit testing framework, and I believe that strongly enough that I'd rather stick with qunit and not implement jest rather than having both. I just happen to think that jest is a better fit for where this extension is headed.

Jest vs QUnit is interesting. Jest appears to be the # 1 JS testing framework outside of mediawiki, with QUnit not even in the top 10. But QUnit is the biggest by far in the mediawiki ecosystem, and I didn't even know Jest was used in mediawiki before I did a codesearch for it.

I am undecided, and created this ticket to help us discuss which one we should pick. And I leave the decision up to the mod tools team, since y'all will be the ones writing the tests. Sounds like you're reluctantly picking QUint?

One of our goals should be to migrate PageTriage to some of the most common MediaWiki technologies in use.

If that's the case, then vanilla vue.js itself could be ruled out, as ooui is far more commonly implemented. An argument could be made to move to codex (the new vue.js component library that implements the current design system standards), but that would make the most sense if we're allowed to change the styling of the components, since the whole point of the library is to follow the current ui standards which the current interface does not implement.

Myself and enwiki NPPers probably do not want changes to PageTriage's visual appearance, so that constrains us a bit. I think "One of our goals should be to migrate PageTriage to some of the most common MediaWiki technologies in use." is a good axiom to follow for almost all of our refactoring, but the front end is a one-off exception.

The reason Vue.js seems like a good choice is Codex was built with it, so it appears to be in the mediawiki stack, and is modern. I'm open to other ideas if you have something else in mind. Perhaps we can discuss a bit in T208256: Pick PageTriage JavaScript/front end rewrite frameworks

To make things more confusing, T305515: Consider migrating from Jest to Vitest is also a possibility :)

My two cents:

It's possible to run both test frameworks via npm run test, GrowthExperiments has for example "test": "grunt test && npm run test:jest" in packakge.json

One of our goals should be to migrate PageTriage to some of the most common MediaWiki technologies in use.

If that's the case, then vanilla vue.js itself could be ruled out, as ooui is far more commonly implemented. An argument could be made to move to codex (the new vue.js component library that implements the current design system standards), but that would make the most sense if we're allowed to change the styling of the components, since the whole point of the library is to follow the current ui standards which the current interface does not implement.

Myself and enwiki NPPers probably do not want changes to PageTriage's visual appearance, so that constrains us a bit. I think "One of our goals should be to migrate PageTriage to some of the most common MediaWiki technologies in use." is a good axiom to follow for almost all of our refactoring, but the front end is a one-off exception.

The reason Vue.js seems like a good choice is Codex was built with it, so it appears to be in the mediawiki stack, and is modern. I'm open to other ideas if you have something else in mind. Perhaps we can discuss a bit in T208256: Pick PageTriage JavaScript/front end rewrite frameworks

I think overall we want to have a consistent UI across features deployed on-wiki, so I am not sure that the one-off design aesthetics of PageTriage can or should be around forever. In addition to UI standardization, we would also benefit from underlying code/library standardization by using Codex, which means easier access to support and improvements to components. But yeah, we should continue that discussion in T208256.

To make things more confusing, T305515: Consider migrating from Jest to Vitest is also a possibility :)

that would be awesome; I think vitest would be a fine choice too, if we can figure out how to choose it.

My two cents:

  • Continue to maintain the QUnit test for the existing non-Vue code

One of the ideas we had (https://en.wikipedia.org/wiki/Wikipedia:Page_Curation/2023_Moderator_Tools_project/Technical_prioritisation) was to greatly expand js unit test coverage. So, this would be in the context of greatly driving up test coverage.

Our thinking is that covering the existing interface with tests will make moving to vue a much higher confidence process, since there would be a suite of tests to provide very clear guidance on how everything should behave. I certainly couldn't +2 a vue patch with confidence right now, because I couldn't be confident that it does what it's supposed to do.

It's possible to run both test frameworks via npm run test, GrowthExperiments has for example "test": "grunt test && npm run test:jest" in packakge.json

I totally get that it's possible, but I think it's a bad idea for aforementioned reasons.

One of our goals should be to migrate PageTriage to some of the most common MediaWiki technologies in use.

If that's the case, then vanilla vue.js itself could be ruled out, as ooui is far more commonly implemented. An argument could be made to move to codex (the new vue.js component library that implements the current design system standards), but that would make the most sense if we're allowed to change the styling of the components, since the whole point of the library is to follow the current ui standards which the current interface does not implement.

Myself and enwiki NPPers probably do not want changes to PageTriage's visual appearance, so that constrains us a bit. I think "One of our goals should be to migrate PageTriage to some of the most common MediaWiki technologies in use." is a good axiom to follow for almost all of our refactoring, but the front end is a one-off exception.

The reason Vue.js seems like a good choice is Codex was built with it, so it appears to be in the mediawiki stack, and is modern. I'm open to other ideas if you have something else in mind. Perhaps we can discuss a bit in T208256: Pick PageTriage JavaScript/front end rewrite frameworks

I think overall we want to have a consistent UI across features deployed on-wiki, so I am not sure that the one-off design aesthetics of PageTriage can or should be around forever. In addition to UI standardization, we would also benefit from underlying code/library standardization by using Codex, which means easier access to support and improvements to components. But yeah, we should continue that discussion in T208256.

I was actually not trying to retry the choice of user interface here, rather I'm trying to understand how our different goals/needs are supposed to fit together. It sounds like the desire is to reduce the "uniqueness" of the extension where possible, but that the ui just happens to be one really big exception/hangup to that.

@kostajh, given that moderator tools is going to be on this for a couple of months, we thought it would make sense to focus on testing and documentation so that others can work on this extension with lower startup cost and greater confidence. If you wanted to, you could weigh in on our ideas here https://en.wikipedia.org/wiki/Wikipedia:Page_Curation/2023_Moderator_Tools_project/Technical_prioritisation

To make things more confusing, T305515: Consider migrating from Jest to Vitest is also a possibility :)

that would be awesome; I think vitest would be a fine choice too, if we can figure out how to choose it.

My two cents:

  • Continue to maintain the QUnit test for the existing non-Vue code

One of the ideas we had (https://en.wikipedia.org/wiki/Wikipedia:Page_Curation/2023_Moderator_Tools_project/Technical_prioritisation) was to greatly expand js unit test coverage. So, this would be in the context of greatly driving up test coverage.

Our thinking is that covering the existing interface with tests will make moving to vue a much higher confidence process, since there would be a suite of tests to provide very clear guidance on how everything should behave. I certainly couldn't +2 a vue patch with confidence right now, because I couldn't be confident that it does what it's supposed to do.

Ah, I see. Another angle to consider is that writing browser tests (webdriverio) might help with this, because it would verify existing functionality at a high level. The QUnit tests you'd write for existing JS code in the repo wouldn't be relevant to future Vue-based extension code, because so much of PageTriage's JS is tied to Backbone and Underscore, which I don't think would make sense to use with Vue.

In GrowthExperiments, we experimented for a while with writing node-qunit tests for non-Vue code, but it was quite painful to do due to the amount of mocking necessary, and I suspect that writing Jest tests for the existing JS code might have the same issues. Maybe it's worth attempting a test or two before committing one way or another?

Maybe it's worth attempting a test or two before committing one way or another?

Good idea! I'll put together a proof of concept patch referencing this task. I probably should have started there.

Okay, I'm synthesizing what's been voiced here and on the ideas/priorities page. I'm going to put a pin in replacing the existing qunit tests for this initial exploration. Instead, I'm going to try writing up a few jest tests for one of the view modules to cover some of the backbone code. If I can get a couple of sensible tests done, I'll also do a followup patch that factors out some of that tested backbone code with vanilla es6 with tests updated as needed. I think I'll learn pretty quickly if my preferred workflow is going to fit here or not.

jsn.sherman renamed this task from Switch Javascript unit tests from QUnit to Jest? to Investigate switching Javascript unit tests from QUnit to Jest [8hrs].Apr 18 2023, 2:32 PM
jsn.sherman claimed this task.
Restricted Application changed the subtype of this task from "Task" to "Spike". · View Herald TranscriptApr 18 2023, 2:32 PM

Change 910002 had a related patch set uploaded (by Jsn.sherman; author: Jsn.sherman):

[mediawiki/extensions/PageTriage@master] Investigate adding jest test for list stats nav

https://gerrit.wikimedia.org/r/910002

I went ahead and pushed the patch as it stands. At the moment, it's not working, but I think it's close? I started with the list stats nav view. It took me a little while to figure out why I couldn't access the constructor, and I realized it was because the view was wrapped in the jquery document ready function. I think this is unnecessary since (so far as I can tell) backbone views create their markup outside the dom. I'm still missing something to get the templates loaded for the view though:

$ npm run jest

[...]

 FAIL  modules/ext.pageTriage.views.list/ext.pageTriage.listStatsNav.test.js
  ● Test suite failed to run

    Template listStatsNav.underscore not found in module ext.pageTriage.views.list

      4 | 	mw.pageTriage.ListStatsNav = Backbone.View.extend( {
      5 | 		tagName: 'div',
    > 6 | 		template: mw.template.get( 'ext.pageTriage.views.list', 'listStatsNav.underscore' ),
        | 		                      ^
      7 |
      8 | 		initialize: function ( options ) {
      9 | 			const that = this;

      at Object.get (node_modules/mock-mediawiki/lib/mediawiki.template.js:94:11)
      at get (modules/ext.pageTriage.views.list/ext.pageTriage.listStatsNav.js:6:25)
      at Object.<anonymous> (modules/ext.pageTriage.views.list/ext.pageTriage.listStatsNav.js:117:2)
      at Object.require (jest.setup.js:4:30)

Test Suites: 1 failed, 1 total
Tests:       0 total
Snapshots:   0 total
Time:        0.865 s
Ran all test suites.

I realized that this extension provides a template compiler for underscore. I'm trying to import it now, but I'm unfamiliar with how this part of mediawiki works. I'm still seeing the same underlying issue:

$ npm run jest

[...]

 FAIL  modules/ext.pageTriage.views.list/ext.pageTriage.listStatsNav.test.js
  ● Test suite failed to run

    Template listStatsNav.underscore not found in module ext.pageTriage.views.list

      92 | 			moduleTemplates = mw.templates.get( moduleName );
      93 | 			if ( !moduleTemplates || moduleTemplates[ templateName ] === undefined ) {
    > 94 | 				throw new Error( 'Template ' + templateName + ' not found in module ' + moduleName );
         | 				      ^
      95 | 			}
      96 |
      97 | 			// Compiled and add to cache

      at Object.get (../../resources/src/mediawiki.template.js:94:11)
      at get (modules/ext.pageTriage.views.list/ext.pageTriage.listStatsNav.js:6:25)
      at Object.<anonymous> (modules/ext.pageTriage.views.list/ext.pageTriage.listStatsNav.js:117:2)
      at Object.require (jest.setup.js:15:37)

Test Suites: 1 failed, 1 total
Tests:       0 total
Snapshots:   0 total
Time:        1.83 s
Ran all test suites.

I think I could probably use some help understanding how client side templating works in mediawiki to get past this. I have a feeling that I'm doing something silly that's not directly related to testing at all.

Okay, I got the testing setup, and really backbone was the cause of all of my troubles. I've got one smoke test in place. The next step would be to turn that into a little bit more useful test, but I've hit my 8 hours of spike time so I'm going to park this until we have some more discussion.

@Novem_Linguae and I did some exploration of qunit today, in which I realized that I hadn't been running qunit locally, *and it wasn't running in CI* edit: it was totally running in ci.

I also have been reading up on vitest as mentioned by @kosta and found that it offers a (mostly) jest compatible api and it has been designed to be easy to migrate from jest.
https://vitest.dev/guide/migration.html

jsn.sherman renamed this task from Investigate switching Javascript unit tests from QUnit to Jest [8hrs] to Investigate switching Javascript unit tests from QUnit to Jest [12hrs].Apr 24 2023, 9:01 PM
jsn.sherman updated the task description. (Show Details)

One reason the qunit test setup has been totally baffling me was its use of headless chrome to create a browser context. Having written qunit tests before, I thought I'd be good to go, but the setup for running qunit via node (which is what I've touched before) is pretty different from running it in a browser. It never occurred to me that a module installed via npm would not execute its unit tests via npm test. This was a good reminder to always go back and check the docs before doing anything in mediawiki!

I realized I didn't say it explicitly: one of the advantages of the browser qunit setup is that it makes impure/poorly isolated code more testable where it would otherwise be difficult. This is actually a great reason to leave the current tests in place for the code they cover.

I realized I didn't say it explicitly: one of the advantages of the browser qunit setup is that it makes impure/poorly isolated code more testable where it would otherwise be difficult. This is actually a great reason to leave the current tests in place for the code they cover.

+1, the integration with other components / lack of isolation from broader MW code is a strength in this context.

Yeah, I really appreciate everyone's feedback on this along the way. I still needed to get my hands dirty to internalize/learn it better, but it was really useful to have the context going in. This experience has definitely impacted my view of our priorities, and I'm thinking about the order that we want to try to tackle things in.

Change 910002 merged by jenkins-bot:

[mediawiki/extensions/PageTriage@master] make jest available for unit tests

https://gerrit.wikimedia.org/r/910002