Spotted by @LucasWerkmeister as Toolforge offers the Gerrit version (which happens to be quite broken, there is an important bugfix that happened after the fork).

Notified the author at WSOAuth#5 (couldn't figure out if they have a username here).

CCing @Xxmarijnw who seems to be the developer on GitHub.

This is not an issue with Gerrit, but rather a mistake on my part. I requested a new Gerrit repository to be forked from the current repository a while back (see here), but I sort of forgot about it and continued to push updates to GitHub.

I hadn't realized the Gerrit repository had received these community translations and updates. It would be great to merge the two remotes and continue to develop the extension on Gerrit exclusively. I am not familiar enough with Git to know how to do this properly, so any help would be greatly appreciated. If I am not mistaken, there would not be any merge conflicts, so I think this would be quite easy.

My sincere apologies for the confusion this fork caused and I hope we can fix it as soon as possible.

(We also have https://github.com/wikimedia/mediawiki-extensions-WSOAuth which is the Github mirror of the Gerrit repository :)

We could always just import (mass cherry-pick / rebase) the Github commits to the Gerrit master, but maybe @QChris or @Reedy have other ideas?

Change 630534 had a related patch set uploaded (by Hashar; owner: Hashar):
[mediawiki/extensions/WSOAuth@master] Merge changes from Github

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

One issue with cherry-picking is that it changes the commits metadata and the sha1 that can be troublesome sometime. Same for a rebase which is essentially the same thing. A merge of the master branch of the Github repository into the master branch of the Gerrit repository. That is what I am proposing with https://gerrit.wikimedia.org/r/c/mediawiki/extensions/WSOAuth/+/630534

The way I did it:

Clone the repositories locally:

git clone https://gerrit.wikimedia.org/r/mediawiki/extensions/WSOAuth
cd WSOAuth
git remote add --fetch github https://github.com/WikibaseSolutions/WSOAuth

Inspect the topology:

git log --oneline --all --graph
* bbf07b8 (HEAD -> master, origin/master, origin/HEAD) Localisation updates from https://translatewiki.net.
* 096bcde build: Updating grunt to 1.3.0
* b940dc8 Localisation updates from https://translatewiki.net.
* 82d2a64 Localisation updates from https://translatewiki.net.
* 79655a3 i18n: Remove empty ExtensionMessagesFiles file
* 2f038d2 Localisation updates from https://translatewiki.net.
* 38c678d Localisation updates from https://translatewiki.net.
* 24ff76d build: Add php-parallel-lint, MinusX and banana-checker
* b1a0ecc Localisation updates from https://translatewiki.net.
* 38d1e46 Localisation updates from https://translatewiki.net.
* 775c7e7 Localisation updates from https://translatewiki.net.
* 69690fe Localisation updates from https://translatewiki.net.
* ef0fe34 Localisation updates from https://translatewiki.net.
* d08a5c7 Localisation updates from https://translatewiki.net.
* 5bcde69 Localisation updates from https://translatewiki.net.
* 955609a Fix message documentation in qqq.json
* aaf139d (origin/REL1_35) Add .gitreview
| * fb4870f (github/master) Add hook to define new authentication handlers outside of WSOAuth folder
| * 00d066b Allow consumer to specify callback URI for applications that have prefixed callbacks enabled (#4)
| * a4462d0 Add maintenance script to manually migrate all or specific users (#3)
| * aed7ff0 Update version number
| * eff36d2 Explicitly set $id reference
| * da4bc86 Merge pull request #2 from kkurzhal/master
|/| 
| * 8a91dff Fixing issue where the user ID will always be null and trigger an exception when checking if the user exists and has not logged in through OAuth.
|/  
* feeaac6 Add option to protect against account usurpation

When I try to merge (git merge github/master) the only conflict I got is in extension.json. The reason is 79655a353840ae0e36eddda8bf245e1dbfcfefba which did:

-  "ExtensionMessagesFiles": {
-    "WSOAuthMagic": "i18n/WSOAuth.i18n.php"
-  },

And also reformatted the file from spaces to tabs. That is automatically handled by git using the ours merge strategy:

git merge -Xours github/master

The resulting merge commit would have be rejected by Gerrit since it does not know about the commits made in Github. I thus imported them:

• created a new github branch in gerrit based on the last common commits between the two repositories: git merge-base origin/master github/master
• Granted myself the rights to Push and to Forge committer identities
• imported the Github master branch in Gerrit: git push origin github/master:refs/heads/github

As a result Gerrit now knows about all the commits made in Github. The only things left is to merge and send for review:

• git merge -Xours github/master
• git push origin HEAD:refs/for/master

Change 630539 had a related patch set uploaded (by Hashar; owner: Hashar):
[integration/config@master] WSOAuth requires composer for mediawiki/oauthclient

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

Change 630539 merged by jenkins-bot:
[integration/config@master] WSOAuth requires composer for mediawiki/oauthclient

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

I have made CI to install the WSOAuth composer dependencies.

Made a change to explicitly mark returns value for setUp and tearDown methods, which is required by PHPUnit 8: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/WSOAuth/+/630538 . That now fails with:

...
1) SpecialPageFatalTest::testSpecialPageDoesNotFatal with data set "PluggableAuthLogin" (PluggableAuthLogin Object (...))
Undefined index: scheme

/workspace/src/vendor/mediawiki/oauthclient/src/ClientConfig.php:77
/workspace/src/extensions/WSOAuth/src/AuthenticationProvider/MediaWikiAuth.php:116
/workspace/src/extensions/WSOAuth/src/AuthenticationProvider/MediaWikiAuth.php:32
/workspace/src/extensions/WSOAuth/src/WSOAuth.php:189
/workspace/src/extensions/WSOAuth/src/WSOAuth.php:40
/workspace/src/extensions/PluggableAuth/includes/PluggableAuth.php:51
/workspace/src/extensions/PluggableAuth/includes/PluggableAuthLogin.php:33
/workspace/src/tests/phpunit/includes/specials/SpecialPageExecutor.php:107
/workspace/src/tests/phpunit/includes/specials/SpecialPageExecutor.php:37
/workspace/src/tests/phpunit/structure/SpecialPageFatalTest.php:43
/workspace/src/tests/phpunit/MediaWikiIntegrationTestCase.php:437
/workspace/src/maintenance/doMaintenance.php:107
=== Logs generated by test case
[objectcache] [debug] MainWANObjectCache using store {class} {"class":"EmptyBagOStuff"}
[localisation] [debug] LocalisationCache using store LCStoreNull []
[objectcache] [debug] MainWANObjectCache using store {class} {"class":"EmptyBagOStuff"}
[localisation] [debug] LocalisationCache::isExpired(en): cache missing, need to make one []
[PluggableAuth] [info] In execute() {"private":false}
[PluggableAuth] [info] Getting PluggableAuth singleton {"private":false}
[PluggableAuth] [info] Class name: WSOAuth {"private":false}
===

2) MediaWikiAuthTest::testIsClientInstanceOfMediaWikiOAuthClientClient
Undefined index: scheme

/workspace/src/vendor/mediawiki/oauthclient/src/ClientConfig.php:77
/workspace/src/extensions/WSOAuth/src/AuthenticationProvider/MediaWikiAuth.php:116
/workspace/src/extensions/WSOAuth/tests/phpunit/MediaWikiAuthTest.php:25
/workspace/src/maintenance/doMaintenance.php:107

3) WSOAuthTest::testAuthenticationProviderIsInstanceOfAuthProvider
Undefined index: scheme

/workspace/src/vendor/mediawiki/oauthclient/src/ClientConfig.php:77
/workspace/src/extensions/WSOAuth/src/AuthenticationProvider/MediaWikiAuth.php:116
/workspace/src/extensions/WSOAuth/src/AuthenticationProvider/MediaWikiAuth.php:32
/workspace/src/extensions/WSOAuth/src/WSOAuth.php:189
/workspace/src/extensions/WSOAuth/tests/phpunit/WSOAuthTest.php:29
/workspace/src/maintenance/doMaintenance.php:107

ERRORS!
Tests: 2559, Assertions: 11164, Errors: 3.

To be honest, I think the current suite of unit tests is useless and they might as well be removed.

These tests likely fail because $wgOAuthUri is an empty string in the testing environment. This causes parse_url() to return an associative array without the required scheme key. That in turn causes the unit test to fail.

Relying on unknown global state in a unit test is of course terrible. No actual logic is tested in the current suite.

What would be the best approach to move on from here? Writing proper tests would be great, but it is out of the scope of this task. The current code base needs to be rewritten to be testable and doing that now would make the move to Gerrit much more cumbersome.

I think ignoring or removing the current test suite, then merging all the changes to the Gerrit master and archiving the GitHub repository would be the best approach.

Put a commit ontop marking the tests as broken (so they're not run), and file a task for fixing them?

As otherwise they will get drive by fixes, or LibUp patches, and will result in people filing bugs about it etc

Indeed the issue is that we have:

{
  "config": {
        "OAuthUri": {
            "value": false,
            "descriptionmsg": "wsoauth-uri-desc"
        },
        "OAuthAuthProvider": {
            "value": "mediawiki",
            "descriptionmsg": "wsoauth-auth-provider-desc"
        },
  }
}

And the false value is used as-is when the client is created. Maybe the extension could be made to not authenticate anything until it is configured?

In T263955#6497434, @hashar wrote:

One issue with cherry-picking is that it changes the commits metadata and the sha1 that can be troublesome sometime.

Cherry-pick and then force push the results to the source repo to keep it in sync? It's worth it for having a clean git history IMO.

In T263955#6498624, @Xxmarijnw wrote:

These tests likely fail because $wgOAuthUri is an empty string in the testing environment.

If that's the only issue with the tests, just set it? Make sure those tests subclass MediaWikiIntegrationTestCase, then add $this->setMwGlobals( 'wgOAuthUri', '...' ); in the setUp method.

@Xxmarijnw: Would you have time to follow the recommendations above?

@Aklapper Would it be possible to remove the current test suite? I looked at doing this before with a colleague, but we could not figure out how to lay a commit on top of an unmerged commit in Gerrit. After that, I forgot about it. I do not have time currently to rewrite the tests and using $this-setMwGlobals('wgOAuthUri', '...'); seems like a bodge to me.

Having a test override the relevant environment parameters is a rather common technique in automated testing. That said, I don't think you need anyone's permission to remove tests from an extension you maintain.

I was not asking for permission, but rather wanted to make sure it was an OK thing to do from a DevOps perspective.

There were more problems with the current test suite, so I decided to still remove the tests.

I created a new patchset for 630534. I am new to Gerrit, so I hope I did it correctly. Can someone more knowledgeable than me verify this?

@Xxmarijnw: Hi, see the jenkins-bot output on that Gerrit patch about which additional fixes are needed.

@Aklapper I have uploaded a new patchset that fixes the error generated by PHPUnit.

The current build is still failing because Selenium is reporting errors. These errors are related to the test user not being able to log in to the wiki. This is to be expected, since WSOAuth is not configured. I am not sure how these errors could be fixed. Can those specific tests be disabled?

Change 643516 had a related patch set uploaded (by Hashar; owner: Hashar):
[integration/config@master] Disable selenium jobs for WSOAuth extension

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

Change 643516 merged by jenkins-bot:
[integration/config@master] Disable selenium jobs for WSOAuth extension

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

The current build is still failing because Selenium is reporting errors. These errors are related to the test user not being able to log in to the wiki. This is to be expected, since WSOAuth is not configured. I am not sure how these errors could be fixed. Can those specific tests be disabled?

Indeed for that extension the mediawiki/core tests will definitely fails since they would fail to login :] I have dropped the related CI job. The merge change from Github ( https://gerrit.wikimedia.org/r/c/mediawiki/extensions/WSOAuth/+/630534 ) can now be voted Code-Review +2 and CI should merge it after jobs have passed :]

Change 630534 merged by Xxmarijnw:
[mediawiki/extensions/WSOAuth@master] Merge changes from Github

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

It looks like this task has mostly been resolved. The only issue that still remains is that the REL1_35 branch still seems to have the old version. How can this be fixed?

Easiest way is to use the cherry-pick option in Gerrit (triple-dot icon on the top right).