Page MenuHomePhabricator

Future of tests/phan in MW Core
Closed, ResolvedPublic

Description

From @Daimona's comments on https://gerrit.wikimedia.org/r/c/mediawiki/core/+/637892/

I guess this wouldn't hurt, but two important remarks:

  • taint-check doesn't check any XML-related methods; is there any security check that does?
  • most importantly, is this file really useful? It seems like a big hack that might be replaced with a custom phan plugin. Furthermore, it's only used by tests/phan/bin/phan, which again seems just an old wrapper, with lots of legacy stuff, outdated options, and whatnot (as a case in point, it tries loading a config file from tests/phan/config.php, but it's been in ./phan/config.php since last year).

Nowadays you only really need 'vendor/bin/phan -d . --long-progress-bar'. We might create a 'composer phan' wrapper, but a dedicated script really seems a horrible overkill that will eventually stop working.

TL;DR: Can we just burn this file (and tests/phan/bin/phan) with fire instead?

Event Timeline

A bit of historic context (from Reedy's comment on that same patch):

Plausibly the directory can be nuked. The post-process script certainly isn't needed anymore, that script implemented per-line issue suppression before phan had it built in. Similarly the bin/phan script dealt with complications like having the entire mediawiki ecosystem in php 5, but running phan analysis from php 7. This again isn't an issue to be concerned with anymore.

I mostly left it around because something (I don't remember what) was still relying on it. At this point it's probably best to just get rid of it and fix whatever fallout, if any, emerges.

Change 641437 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/core@master] Kill tests/phan

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

@EBernhardson @Legoktm Thank you for the input, I've nuked the directory. A quick and hacky search doesn't show any usage.

Change 641437 merged by jenkins-bot:
[mediawiki/core@master] Kill tests/phan

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

Change 641301 had a related patch set uploaded (by Reedy; owner: Daimona Eaytoy):
[mediawiki/core@REL1_35] Kill tests/phan

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

Change 641301 merged by jenkins-bot:
[mediawiki/core@REL1_35] Kill tests/phan

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