Page MenuHomePhabricator
Create Task
Maniphest T202743

Add precommit tests to MobileFrontend repositories
Closed, ResolvedPublic
Actions

Description

This task encompasses the work to add a pre-commit or Husky precommit hook to exercise any fast running tests prior to commit. The configuration for the former is npm i -D pre-commit the configuration for the latter is npm i -D husky and adding a "precommit": "npm -s t" script. As part of this task, the assignee should task-ify porting any other tests from makefile-installed hooks to the new system.

Precursor

To be discussed in a future CR session:

What scripts do we want to maintain in MobileFrontend?
https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/extensions/MobileFrontend/+/master/dev-scripts/ [can be removed] (although validatehtml.sh may still be of interest
https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/extensions/MobileFrontend/+/master/Makefile
https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/extensions/MobileFrontend/+/master/composer.json
https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/extensions/MobileFrontend/+/master/package.json
https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/extensions/MobileFrontend/+/master/Rakefile [Needed for Ruby browser tests]
https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/extensions/MobileFrontend/+/master/Gruntfile.js

Details

Related Changes in Gerrit:
SubjectRepoBranchLines +/-
Drop MakeFile and friendsmediawiki/extensions/MobileFrontendmaster+8 -426
Precommit hook should run `composer test` and `npm run test:unit`mediawiki/extensions/MobileFrontendmaster+2 -0
Introduce pre-commit npm modulemediawiki/extensions/MobileFrontendmaster+1 -0
Customize query in gerrit

Related Objects
Search...

Event Timeline

• Niedzielski created this task.Aug 24 2018, 3:09 PM
• Niedzielski triaged this task as High priority.
Jdlrobson edited projects, added Web-Team-Backlog-Archived; removed Web-Team-Backlog-Archived (Readers-Web-Kanbanana-Board-2018-19-Q1).Aug 24 2018, 4:02 PM
Jdlrobson moved this task from Incoming to Needs Prioritization on the Web-Team-Backlog-Archived board.Aug 24 2018, 4:53 PM
Jdlrobson edited projects, added MobileFrontend (MobileFrontend and MinervaNeue architecture); removed Epic, MobileFrontend.
Jdlrobson renamed this task from Add precommit tests to Add precommit tests to Minerva and MobileFrontend repositories.Aug 27 2018, 11:45 PM
Jdlrobson lowered the priority of this task from High to Medium.
Jdlrobson updated the task description. (Show Details)
Jdlrobson moved this task from Backlog to Code coverage on the MobileFrontend (MobileFrontend and MinervaNeue architecture) board.Aug 30 2018, 1:32 AM
Jdlrobson subscribed.Sep 10 2018, 4:50 PM

Stephen wants less entry points.
Minerva has a pre commit hook which runs npm test using pre-commit node package (just like Popups)
Rakefile removal is blocked on porting our browser tests to Node.js
Makefile is super old and probably lived out its lifecycle

We've decided to:

  • Add a replacement package.json Node.based precommit hook
  • Drop Makefile, but check the make docs isn't needed to generate docs.wikimedia.org
  • Review remaining files, reconvene to discuss about porting those.
• Niedzielski added a subscriber: Jdrewniak.Sep 10 2018, 7:21 PM

Stephen wants less entry points.

Minimize tooling to what is used and specific to MobileFrontend, other tooling should be removed or moved to another repo. Minimize where tooling lives, usually scripts in package.json.

Rakefile removal is blocked on porting our browser tests to Node.js

@Jdrewniak, is this MinervaNeue Ruby port patch ready for merging? Can we do the same for MobileFrontend?

Add a replacement package.json Node.based precommit hook

npm i -D pre-commit per a similar commit in MinervaNeue + removing dev-scripts and Makefile code. The author should also consider making a similar patch in Popups for consistency.

make docs isn't needed to generate docs.wikimedia.org

There are two jobs, mwext-doxygen-publish and mwext-npm-doc-publish. The former is used for PHP and runs Doxygen directly, not make.

Makefile is super old and probably lived out its lifecycle

Whoever picks this up should be aggressive in hosing out the Makefile and dev-scripts.

Jdlrobson renamed this task from Add precommit tests to Minerva and MobileFrontend repositories to Add precommit tests to MobileFrontend repositories.Sep 11 2018, 10:03 PM

(Minerva has one)

gerritbot subscribed.Sep 11 2018, 10:17 PM

Change 459853 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Introduce pre-commit npm module

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

gerritbot added a project: Patch-For-Review.Sep 11 2018, 10:17 PM

Change 459854 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Drop MakeFile and friends

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

gerritbot added a comment.Sep 11 2018, 11:58 PM

Change 459853 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Introduce pre-commit npm module

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

ReleaseTaggerBot added a project: MW-1.32-notes (WMF-deploy-2018-09-18 (1.32.0-wmf.22)).Sep 12 2018, 12:00 AM
gerritbot added a comment.Sep 12 2018, 8:27 PM

Change 460093 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Precommit hook should run composer test and npm run test:unit

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

gerritbot added a comment.Sep 13 2018, 5:32 PM

Change 460093 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Precommit hook should run composer test and npm run test:unit

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

Jdlrobson added a comment.Sep 13 2018, 6:16 PM

Patches are merged. Anything left to do here or can we resolve?
Should we open separate tasks for the HTML validation? SVG checking (seems to be captured in T204260)?

• Niedzielski added a comment.Sep 13 2018, 6:23 PM

@Jdlrobson, I've had time to reflect since our meeting and I wonder if the HTML validator is worth maintaining. For me personally, it's not. I'm not sure if it's a mistake but I haven't been using it and would use the web UI in a pinch. What do you think of removing it for now and resurrecting it from version control if needed later?

gerritbot added a comment.Sep 13 2018, 7:11 PM

Change 459854 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Drop MakeFile and friends

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

Jdlrobson removed a project: Readers-Web-Kanbanana-Board-Old.Sep 17 2018, 5:11 PM
Jdlrobson closed this task as Resolved.Sep 19 2018, 11:57 PM
Jdlrobson claimed this task.

Marking this as resolved.
I've submitted a patch for removing the HTML validator. Let's open new tasks as necessary, given precommit hook support now exists.
https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/MobileFrontend/+/461497 Remove validate HTML dev-script

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits