Page MenuHomePhabricator

`composer lint` doesn't actually work in libraryupgrader on mw core
Closed, ResolvedPublic

Description

It was noticed when playing with LibUp

> composer lint
Cannot create cache directory /cache/composer/repo/https---packagist.org/, or directory is not writable. Proceeding without cache
Cannot create cache directory /cache/composer/files/, or directory is not writable. Proceeding without cache
> parallel-lint --exclude vendor
PHP Parallel Lint version 0.9.1
-------------------------------
Usage:
    parallel-lint [sa] [-p php] [-e ext] [-j num] [--exclude dir] [files or directories]

Options:
    -p <php>        Specify PHP-CGI executable to run (default: 'php').
    -s, --short     Set short_open_tag to On (default: Off).
    -a, -asp        Set asp_tags to On (default: Off).
    -e <ext>        Check only files with selected extensions separated by comma.
                    (default: php,php3,php4,php5,phtml)
    --exclude       Exclude directory. If you want exclude multiple directories, use
                    multiple exclude parameters.
    -j <num>        Run <num> jobs in parallel (default: 10).
    --no-colors     Disable colors in console output.
    --json          Output results as JSON string (require PHP 5.4).
    --blame         Try to show git blame for row with error.
    --git <git>     Path to Git executable to show blame message (default: 'git').
    --stdin         Load files and folder to test from standard input.
    --ignore-fails  Ignore failed tests.
    -h, --help      Print this help.

It just prints the help, it doesn't actually lint anything in mw core (as expected)

Extensions generally have the '.' in there...

parallel-lint . --exclude vendor --exclude node_modules

Event Timeline

Change 586443 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@master] Make composer lint actually lint files

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

Reedy renamed this task from `composer lint` doesn't actually work to `composer lint` doesn't actually work in libraryupgrader on mw core.Apr 6 2020, 9:15 PM
Reedy added a project: LibUp.
Reedy updated the task description. (Show Details)

Change 586443 abandoned by Reedy:
Make composer lint actually lint files

Reason:
Apparently this is basically a perf hack?

Hmm...

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

If we call composer test .... and it results in parallel-lint . --exclude vendor --exclude node_modules '.' in extensions.. Would it be linting twice?

If we call composer test .... and it results in parallel-lint . --exclude vendor --exclude node_modules '.' in extensions.. Would it be linting twice?

🤷🏽‍♂️

If we call composer test .... and it results in parallel-lint . --exclude vendor --exclude node_modules '.' in extensions.. Would it be linting twice?

Testing on a smaller repo (IPUtils)

$ composer lint
> parallel-lint . --exclude vendor
PHP 7.3.11 | 10 parallel jobs
...                                                          3/3 (100 %)


Checked 3 files in 0.2 seconds
No syntax error found
$ composer lint .
> parallel-lint . --exclude vendor '.'
PHP 7.3.11 | 10 parallel jobs
...                                                          3/3 (100 %)


Checked 3 files in 0.1 seconds
No syntax error found
$ composer lint .
> parallel-lint --exclude vendor '.'
PHP 7.3.11 | 10 parallel jobs
...                                                          3/3 (100 %)


Checked 3 files in 0.1 seconds
No syntax error found

Looks like php-parallel-lint does some de-duping

So is it worth doing something like this to do the linting on mw core too?

diff --git a/libup/ng.py b/libup/ng.py
index d3bb8fb..b2e17a4 100755
--- a/libup/ng.py
+++ b/libup/ng.py
@@ -222,7 +222,7 @@ class LibraryUpgrader(shell.ShellMixin):
         if not self.has_composer:
             return
         self.check_call(['composer', 'install'])
-        self.check_call(['composer', 'test'])
+        self.check_call(['composer', 'test', '.'])
 
     def fix_coc(self):
         if not os.path.exists('CODE_OF_CONDUCT.md'):
@@ -588,7 +588,7 @@ class LibraryUpgrader(shell.ShellMixin):
             with open(phpcs_xml, 'w') as f:
                 f.write(text)
             try:
-                subprocess.check_call(['composer', 'test'])
+                subprocess.check_call(['composer', 'test', '.'])
             except subprocess.CalledProcessError:
                 self.log('Tests still failing. Skipping')
                 return False

Change 587243 had a related patch set uploaded (by Reedy; owner: Reedy):
[labs/libraryupgrader@master] Actually lint MW core

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

Change 587243 merged by jenkins-bot:
[labs/libraryupgrader@master] Actually lint MW core

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

Reedy claimed this task.
Legoktm added a subscriber: Legoktm.

Reverted in https://gerrit.wikimedia.org/r/c/labs/libraryupgrader/+/599471 - we need to limit this change to just MediaWiki core's unique setup.

Reedy removed Reedy as the assignee of this task.Apr 20 2021, 12:00 AM

Does test-some help here?