[4.0] Patch tester enhancement for joomla4 (pre-compiled diff)

[eXsiLe95]

Patchtester enhancement for Joomla4

Until Joomla 4, Joomla did not have any assets that needed to be comiled in any way. Therefore, testing pull requests could be made a little more easy for people who are not into dev-stuff and do not have installed any dev-tools (except xampp/MAMP/LAMP... maybe). The patchtester called the Github API and asked for any changes, downloaded those changed files and exchanged it on the client machines (note that it did not only apply the diff; therefore a git client would be needed).

In Joomla 4 with the new front-end, you need to compile the source files with composer and npm before you are good to go with your Joomla instance. As many users don't know how to use these tools, we provide a pre-compiled version of Joomla which the user has to unzip and deploy on his server. There are no tools needed except a (s)ftp client or xampp/MAMP/LAMP...!

Applying "patches" the way patchtester works is possible but very limited. You can't get any compiled files from Github, so you only apply the source files, but not the compiled ones Joomla uses. For this very reason, we came to the conclusion that pre-compiling and deploying this pre-compiled diff is the best solution for now to be able to keep serving the patchtester for our users.

Therefore, we already implemented another step in the Drone CI automation that will search for any diffs in the compiled version of the newest Joomla 4.0-dev-Branch with the branch that the pull request is based on, as you can see in this pull request joomla/joomla-cms: #26274. This step is done by a new, fairly lightweight docker image which is now included in the joomla-projects/docker-images repository.

For the patchtester itself, the following changes were made:

Summary of Changes

Changes visible to the user

• A switch is added where the user can choose where the diff should come from. Per default, since this enhancement is developed for the purpose of testing Joomla 4, the CI server is selected.

• You can specify the location of the CI server in the global settings.

• Since pull requests can build on each other and the patchtester is able to apply multiple pull request "patches", the user is forced to remove the patches in the exact opposite order he applied them. For usability reasons, he is still able to use the reset button on the top of the patchtester to revert all changes. The patchtester will then automatically revert the patches in the correct order, no matter where the changes came from (Github/CI).

• Language strings for en-gb, en-us and de-de were added for the CI mechanism.

Changes inside the "black box"

• Applying and reverting changes was refactored so that different routines are executed when called, depending on whether the Github or CI technique was selected in global configuration.

• For the CI mechanism, new methods to fetch, apply and revert changes where implemented.

• When applying multiple patches via CI, a patch chain was added which keeps which pull requests were added and in which order. The patch chain is stored in a new table #__patchtester_chain. This is necassary to revert the changes correctly without breaking Joomla, which has been an issue on patchtester so far. The patch chain is not used when using Github so far, but could be implemented if wanted.

• The buttons where refactored to event-handling (no more inline-href-javascript).

• The old template overwrites were removed.

Testing Instructions

• Get a fresh Joomla 4 installation. You can get Joomla 4 without any tooling needed here: Joomla Nightly Builds

• Download this PR as zip from this pull request, build the extension using the build script and install it as you would install any other extension in Joomla.

• Search for a pull request which already has a diff build and deployed by Drone CI on the CI server.

• In the backend of Joomla, go to patchtester, update the pull requests using the fetch data button and click on apply at your selected pull request.

• Check if the changes proposed by the selected pull requests where applied.

• Go to https://ci.joomla.org:444/artifacts/joomla/joomla-cms/4.0-dev/<pull-request-id>/patchtester/build.zip and download the zip file with your browser after you inserted the pull request id of you pull request in the corresponding place of the url.

• Open up that zip file. It should contain all new or changed files of the pull request as well as a list of deleted files if there are any in the pull request.

• Check your joomla installation for those files and their file contents.

• Revert the changes using either the revert button in patchtester or by clicking on reset.

• Check your joomla installation again. The original files should be recovered and no files of the applied changes should be there.

• Please feel free to think outside the box when testing this!

[eXsiLe95]

Imo the PHP 5.6 test should be abandoned since the support of PHP5.6 was stopped since end of 2018.

[mbabker]

Imo the PHP 5.6 test should be abandoned since the support of PHP5.6 was stopped since end of 2018.

How about reviewing the task that was executed on PHP 5.6 instead of just flat out saying it should be abandoned? Or, you can just disable PHPCS for this repo then you don't have to worry about CI failing because your code style is not compliant with the Joomla coding standards.

[eXsiLe95]

@datsepp increased the required PHP version to 7.0.x as you can see in composer.json, so the failing test is expected. He just relayed on the PHP recommendation - but if you prefer, we will remove this change and accept PHP5.6. The question is: do we really need to support PHP 5.6 when Joomla 4 already requires PHP 7.2.x?

[alikon]

don't forget that j.3 will be supported till....... and php requirements for j.3 is 5.3.10,
as it seems that there will be only 1 com_patchtester version which should work for both 3.x and 4.x...

[mbabker]

Did anyone actually look at the CI configuration or did you all stop reading at 5.6 and just arbitrarily decide "oh, hey, this PHP version isn't supported anymore, that's a problem"?

Since it seems my question is rhetorical and I already know the answer to this, let me explain how it works.

This repository enforces the Joomla Coding Standards, presently the 1.x version of them, which relies on PHP_CodeSniffer version 1.x. To my knowledge, this version of PHP_CodeSniffer is not supported on any PHP 7 build. So, logically, the coding standards are checked on the newest version of PHP that the version of the tool in use supports, PHP 5.6.

That wasn't hard, was it?

[mbabker]

The question is: do we really need to support PHP 5.6 when Joomla 4 already requires PHP 7.2.x?

Well, since you have decided to rewrite this component in a way that can only support 4.0, then yes, you are free to drop support for older PHP versions, and Joomla 3.x as a whole. Actually, you can probably drop GitHub support as a whole too since it seems you aim to only support builds for the CMS repo coming from Drone, ignoring the fact that this component could actually be used with any GitHub repository and any Joomla extension that follows the CMS file structure. You can probably drop Crowdin support as well as it seems you're now manually updating translation files and not translating all messages that reach the user interface.

[N6REJ]

o my knowledge, this version of PHP_CodeSniffer is not supported on any PHP 7 build. So, logically, the coding standards are checked on the newest version of PHP that the version of the tool in use supports, PHP 5.6.

@mbabker seems it requires 5.4+
https://github.com/squizlabs/PHP_CodeSniffer/wiki/Requirements

[mbabker]

That is the current version of the tool, not the version in use right now.

[SharkyKZ]

Revert.

[SharkyKZ]

Do JS code changes work with whatever browsers J3 supports?

[sebenns]

Do JS code changes work with whatever browsers J3 supports?

They do mostly. IE 8 supports the proprietary .attachEvent method instead,
but there also exists a workaround if necessary so.

Thank you for reviewing~.

[SharkyKZ]

composer.lock needs to be regenerated.

[SharkyKZ]

They do mostly. IE 8 supports the proprietary .attachEvent method instead,
but there also exists a workaround if necessary so.

What about the use of let? Seems it's unsupported https://caniuse.com/#search=let

[SharkyKZ]

Please revert formatting changes in this file.

[sebenns]

What about the use of let? Seems it's unsupported https://caniuse.com/#search=let

Yeah, I saw that yesterday at night as well, had not the time to change it.
Thanks for pointing that out, I replaced this as well as the arrow functions.

...and thanks again for reviewing~.

[alikon]

in the hope to try to have something working without hard debate, or with the goal to have something working for the Worldwide PBF event, despite different opinion.... my silly proposal is to a have an hard fork of com_patchtester....so with that in mind .... i've made a silly fork of com_patchtester + this draft pr that in my humble opinion could be a way to superate discussion and try to do something quite fast to be delivered before PBF......or at least i'm trying to do something i'm convinced could be a good compromise

https://github.com/alikon/com_pbf

the repo is open to everyone....i swear i'll merge all pr's that will be submitted

[Harmageddon]

Some things I ran into when trying to test this:

Go to https://ci.joomla.org/artifacts/joomla-cms/4.0-dev//patchtester/build.zip and download the zip file with your browser after you inserted the pull request id of you pull request in the corresponding place of the url.

The URL seems to be https://ci.joomla.org:444/artifacts/joomla/joomla-cms/4.0-dev/<pull-request-id>/patchtester/build.zip. Can you please correct this in the testing instructions?

I tried to apply joomla/joomla-cms#26210 and joomla/joomla-cms#26360, but both failed when trying to move files like "administrator/components/com_media/package-lock.json". To me, the ZIP files seem to contain many files not directly related to the patch, like the package-lock.json which is write-protected and thus can't be moved or changed. And the amount of files contained in a ZIP seems to grow the older a PR gets (correct me if I'm wrong). For example, for joomla/joomla-cms#26360, I'm getting a 12MB download.

Could this be reduced somehow? Maybe by letting drone attempt to rebase before creating the ZIP, so the ZIP really only contains the changes? (I'm not an expert on CI or NPM, so feel free to correct me if I'm mistaken)

[eXsiLe95]

The URL seems to be https://ci.joomla.org:444/artifacts/joomla/joomla-cms/4.0-dev/<pull-request-id>/patchtester/build.zip. Can you please correct this in the testing instructions?

Will do! Thanks for pointing that out, I missed that one with the last changes. EDIT: Done.

I tried to apply joomla/joomla-cms#26210 and joomla/joomla-cms#26360, but both failed when trying to move files like "administrator/components/com_media/package-lock.json". To me, the ZIP files seem to contain many files not directly related to the patch, like the package-lock.json which is write-protected and thus can't be moved or changed.

You are correct with this one, there are still a lot of files which need to be excluded from building the archives, which I will fix soon.

And the amount of files contained in a ZIP seems to grow the older a PR gets (correct me if I'm wrong). For example, for joomla/joomla-cms#26360, I'm getting a 12MB download.

Could this be reduced somehow? Maybe by letting drone attempt to rebase before creating the ZIP, so the ZIP really only contains the changes? (I'm not an expert on CI or NPM, so feel free to correct me if I'm mistaken)

This would be a possibility, but I need to check how easily the rebase can be automated (if it can not be done automatically)... Maybe the file size of the archive will be reduced with the changes in the compare step aswell.

[Hackwar]

Can you please update this branch to the latest changes? Then I would merge this.

[Hackwar]

Thank you!