I moved my Moodle plugin out of Moodle's tree (and broke git push)
ContenidoContents
For years, my plugin mod_jitsi lived where almost all of them live: inside Moodle itself, in mod/jitsi. It works, but it carries a quiet problem: the plugin is tied to that Moodle checkout. If I want to test it on another version, I have to copy files, or keep duplicated code, or remember to sync by hand. A mess.
So I decided to move it to its own repository, outside Moodle’s tree, and link it with symlinks into the installs where I need it. A single repo, several Moodle versions pointing at the same code. I’d decided to finally sort out that mess of having the plugin tied to a single checkout, so I did a big refactor, git add, git commit, git push… and the push broke.
What looked like harmless plumbing uncovered three things. Let me walk you through them.
One repo, several Moodle versions
The standalone repo idea is simple: the plugin lives in a neutral place, say ~/moodles/plugins/mod_jitsi, and from each Moodle I create a symlink pointing there:
- Moodle 4.5 →
~/moodles/stable_405/moodle/mod/jitsi - Moodle 5.2 →
~/moodles/stable_502/moodle/public/mod/jitsi
(Yes, in Moodle 5.x the code moved under public/. I’ll come back to that below, because it causes trouble.)
The upside is huge: I edit once and the change is in both installs at the same time. I can test the same plugin against two Moodle versions without duplicating anything. And that’s exactly what, without knowing it yet, was about to save me a moment later.
The push that broke
The repo ships a git pre-push hook (in scripts/pre-push) that, before letting anything go up, runs two checks: the code sniffer (phpcs) and the PHPUnit test suite. If something fails, no push. It’s my safety net.
To find PHPUnit, the hook did this:
1PLUGIN_DIR="$(cd "$(dirname "$0")/.." && pwd)"
2MOODLE_ROOT="$(cd "${PLUGIN_DIR}/../.." && pwd)" # assumes mod/jitsi
3PHPUNIT="${MOODLE_ROOT}/vendor/bin/phpunit"
See the ../..? When the plugin lived in mod/jitsi, going up two levels from the plugin landed you right at Moodle’s root, where vendor/bin/phpunit lives. It worked perfectly… as long as the plugin lived there.
But now the repo lives in ~/moodles/plugins/mod_jitsi. Going up two levels from there takes you to ~/moodles, which is not a Moodle: no vendor/bin/phpunit, nothing of the sort. The hook couldn’t find PHPUnit, exited with exit 1 and cancelled the push. (The sniffer passed fine; the only broken thing was locating the tests.)
The underlying lesson already shows here: tooling that assumes the directory layout breaks the moment you change that layout. That ../.. was a time bomb waiting for me to move the plugin somewhere else.
Fix 1: detect instead of assume
I rewrote the detection so the hook looks for a Moodle with PHPUnit, instead of taking for granted where it is. With priorities:
- An environment variable,
MOD_JITSI_MOODLE_ROOT, in case I want to force which install to test against. - The classic
../..layout, in case the plugin ever lives inside a Moodle again. - A list of known installs, preferring the newest version that has PHPUnit installed (5.2 before 4.5).
1detect_moodle_root() {
2 [ -n "$MOD_JITSI_MOODLE_ROOT" ] && [ -f "$MOD_JITSI_MOODLE_ROOT/vendor/bin/phpunit" ] \
3 && { echo "$MOD_JITSI_MOODLE_ROOT"; return 0; }
4 local classic; classic="$(cd "$PLUGIN_DIR/../.." 2>/dev/null && pwd)"
5 [ -f "$classic/vendor/bin/phpunit" ] && [ -f "$classic/version.php" ] \
6 && { echo "$classic"; return 0; }
7 for c in ~/moodles/stable_502/moodle ~/moodles/stable_405/moodle; do
8 [ -f "$c/vendor/bin/phpunit" ] && { echo "$c"; return 0; }
9 done
10 return 1
11}
And a change of attitude that, for me, matters most: if it doesn’t find PHPUnit, the hook now warns and lets the push through instead of cancelling it. Code style is non-negotiable; tests run if they can. Why? Because a hook that blocks you over something that isn’t your code’s fault ends up dead: you disable it with git push --no-verify and, the day it really matters, it no longer protects you. A guard that throws too many false positives is a guard you stop listening to.
Fix 2: testing against two versions at once (and what it uncovered)
Here came the unexpected prize of the standalone repo. Since the same code is symlinked in 4.5 and 5.2, I could run the same test suite against two different environments.
The 4.5 one already had PHPUnit set up. I prepared the 5.2 one (which didn’t yet): composer install, set $CFG->phpunit_prefix and $CFG->phpunit_dataroot in config.php, and initialize the test environment:
1php public/admin/tool/phpunit/cli/init.php
Mind that public/: in Moodle 5.x the PHPUnit CLI lives under public/, not where it was in 4.x. It’s the kind of detail that makes you waste ten minutes looking for a file that “disappeared”.
I ran the plugin’s test suite —over 150 tests— and they all passed in both versions. But on 5.2, PHPUnit finished them off with a warning repeated over and over:
Metadata found in doc-comment for method mod_jitsi\external_test::test_register_push_subscription_creates_record().
Metadata in doc-comments is deprecated and will no longer be supported in PHPUnit 12.
Use attributes instead.
One deprecation per test with metadata in its comment. Since I had one @covers per test, that was a few hundred warnings. Something had changed between one version and the other.
Fix 3: from @covers to #[CoversClass]
The cause was the version gap. Moodle 4.5 ships PHPUnit 9.6 and Moodle 5.2 ships PHPUnit 11.5. And in PHPUnit 11 the docblock metadata (@covers, @dataProvider…) is deprecated in favour of PHP attributes. Since in mod_jitsi I annotate coverage test by test, I had a @covers on nearly every one: hence the flood of warnings.
Before, each test carried its @covers in the comment:
1/**
2 * @covers \mod_jitsi\external\register_push_subscription::execute
3 */
4public function test_register_push_subscription_creates_record(): void { /* ... */ }
I migrated it to a class-level attribute, which is how Moodle core itself writes it in 5.2:
1use PHPUnit\Framework\Attributes\CoversClass;
2
3#[CoversClass(\mod_jitsi\external\register_push_subscription::class)]
4final class external_test extends \advanced_testcase {
5 // ...
6}
And here’s the nice compatibility detail, the one that keeps this from being a headache: PHP attributes (#[...]) have existed since PHP 8.0, but PHPUnit 9.6 doesn’t read them (only PHPUnit 10 and up reflect on them). Since an attribute only “activates” when something reflects on it, on 4.5 with PHPUnit 9.6 they’re simply ignored, with no error.
Translation: the same test file stays clean on 11.5 and keeps passing as-is on 9.6. I don’t need two versions of the tests, conditionals, or tricks. Verified in both places: on 5.2 with PHPUnit 11.5, the whole suite green and zero deprecations; on 4.5 with PHPUnit 9.6, the same tests green and not a single warning.
If you have Moodle plugins, this change will hit you for sure when you jump to 5.x: migrating @covers and friends to attributes is part of the PHPUnit 11 toll. The good news is you can do it without breaking compatibility with the old versions.
The lesson
Moving a Moodle plugin to its own repo and linking it to several versions is a great idea: a single place where the code lives and real tests against 4.5 and 5.2 at once. I’d do it again without hesitation.
But by decoupling the plugin from a specific checkout, the tooling can’t assume anything: not the path (../..), not which PHPUnit version is around, not even that PHPUnit exists. The three fixes fit in one line:
- Detect, don’t assume: look for the Moodle, don’t take its location for granted.
- Degrade, don’t block: warn if the tests are missing, don’t cancel the push.
- Write for the new without breaking the old: attributes that 11.5 understands and 9.6 ignores.
That ../.. looked harmless for years. It was… until I moved a folder. Almost all the tooling we write carries an assumption like that inside, waiting for its moment.