[4.0] Patch tester enhancement for joomla4 (pre-compiled diff)#26247
[4.0] Patch tester enhancement for joomla4 (pre-compiled diff)#26247HLeithner merged 13 commits intojoomla:4.0-devfrom
Conversation
bb1f6ee to
43d664a
Compare
ca3336d to
53e908a
Compare
|
@mbabker Sorry, I did not want to disrespect you and your work in any kind. I'm currently working on an extension of the patch tester, basically. The problem with J4 is that it has compiled files for the front end that can not be tested with Joomla4 without running composer install and npm install. Testers of Joomla don't have those tools, in general. Therefore, the patch tester is not working as intended for Joomla 4. It's not that I would have wanted to misappropriate your work for the compatibility of the patch tester for Joomla 4. To be able to test all the PRs with the patch tester, we (@datsepp @Hackwar and I) are working on the patch tester extension aswell. The plan is to pre-compile the files and store them on the ci server so that the patch tester can apply the pre-compiled files instead of compiling them on the client's machine, where tools would be needed. I'm trying to find a more detailed name of what we are working on for this PR... Sorry 'bout that! EDIT: I updated the description of this PR to get some information rolling on this subject since there is no issue for what we're trying to do here. I hope it gets understandable. If you have any questions or something is not clear, please do not hesitate to ask. |
I'm confused. If your pulling git to test patches then you know you need to run the J!4 build before you can use it, and if your testing as a "user" then you already have a "working" J!4 and have no need for the build process again, as its already been done. |
|
@N6REJ thats the entire point of this code. So that you can test patches as easily in J4 as you could in J3 |
|
Nobody is claiming that it is a 100% replacement. We all know that Patchtester is an imperfect tool and nobody is trying to replace a developer installation with this. But we are trying to make more PRs testable with the Patchtester and make move from 70% testable to 90% testable. The ATT is also not neglecting the test coverage of Joomla. Right now there are 5 people working on migrating old tests to J4 and/or writing new tests. We are also working on stabilising the existing test suites in order to prevent those false negatives that we suffered from lately. Also please don't claim that a user of Patchtester has an idea of which files that component is changing on the system. To the audience of Patchtester, it is a black box either way. |
Co-Authored-By: Harald Leithner <leithner@itronic.at>
…-cms into patch-tester-for-joomla4
|
maybe a silly/naive question.... |
This is partly correct. As we implemented it now (the PR for the patchtester component itself will most likely come tomorrow) you can choose the mechanism the patchtester works with. In the global configuration, you can choose whether to use the common version or get the diff from CI. So basically we are backward compatible, even though we do not focus on that in development. You can't test 3.x patches on a 4.x Joomla and vice versa, so it wouldn't be that big of a mess with two versions of the patchtester either. |
I made a well recieved video series on exactly how to use patch tester from start to finish. The user shouldn't have to jump thru a boat load of hoops just because a bunch of dev's like doing things from dev perspective instead of end-user perspective.
Why should this be necessary? IF the only way it works with 4 is from this mystery CI server then there is no need for it to be present! Normal working way for 3.x and the other for 4. One of the things I hear over and over again is how difficult it is to help with the project. |
|
@eXsiLe95 Am I understanding correctly that this "new" patch tester will REQUIRE docker? |
|
@N6REJ No, this does not require Docker. Why should it? Why do you assume that we are making it any harder at all to test and/or contribute? This is part of an improvement to the patchtester to allow for easier testing for inexperienced users. @mbabker The same way that you can decide what to do with your time, @eXsiLe95 and I can decide what to do with our time. If we decide to invest it into makeing it possible to test JS/CSS changes in 4.0 with the Patchtester, then that is our choice. We could have had a discussion about this, if you wanted to, but you made it pretty clear early on, that you had no interest here, culminating in you dropping that project like a hot potatoe in the second post here. @N6REJ based on the above discussion, I hope that you understand that Michaels complaints are purely theoretical in nature. There is hardly any change visible to the users. |
|
I see no use in any further back and forth between me and @mbabker here, so lets leave it at that. For others, who join this here: The Joomla community has matured together with Joomla and there will be a time when NPM and composer are nothing spectacular anymore, but I'm expecting that to happen only after 4.0 has been released. However, in a few weeks we have the probably largest PBF of Joomla so far and a lot of people are going to be there who will be capable of using Patchtester, but not mess around with NPM and composer. Since this solution here is not too complex, it seems like a good solution at least for the PBF. @eXsiLe95 Please add limitations to the publish-diff step that it is only run for PRs against the 4.0-dev branch of the joomla-cms repo. |
|
@eXsiLe95 and make sure that composer and npm are run for the reference. 😉 |
This file is a docker file is it not? So, if its not requiring docker then I'm totally confused. |
As a user, you will not get in touch with docker at all. You just download J4, the patch tester, install it and you are good to go. Same procedure as it has been before. As a drone ci server, to be able to serve the files needed for that behaviour, we needed to include another step, which indeed is a docker file, which sums up the diff of the pull request and deploys this diff as a zip file on the ci server. But you don't need to touch docker once! |
|
This file is the configuration file for our drone CI system. Unless you want to setup your own Joomla CI server, this is pretty much irrelevant to you. |
tyvm for the clarification. Thats good to hear. Please make sure you don't need to install a different version for J3. the installer can handle that by itself if necessary. |
|
Thank you very much for creating diffs between PRs and latest commit so the patch tester don't composer and npm installed. Thats a first step so more stuff could come soon. |
In this pull request I would like to propose another step in the Drone CI execution pipeline that zips the pre-compiled source files of the current branch compared to a reference branch and uploads this zip to a CI server. In my eyes, this is necessary to give non-developer testers the possibility to test changes/bugfixes/etc. of an pull request within the patch tester and without the need to have developer tools like npm and composer installed. The patch tester will also be adjusted to look for the zip files in a future pull request in its repository (link will follow). The steps performed in this additional CI step can be seen here: joomla-projects/docker-images: patchtester
Summary of Changes
Testing Instructions
Testing instructions differ for the availability of our enhancement of the Patch Tester for Joomla 4.
As long as our enhancement of Patch Tester for Joomla 4 is not available:
SERVER-ADDR.PULL-ID.https://<SERVER-ADDR>/<PULL-ID>/build.ziphttps://<SERVER-ADDR>/<PULL-ID>/deleted-files.logExpected result
build.zipdeleted-files.logWhen our enhancement of Patch Tester for Joomla 4 is available:
For optimal test results, run this on a system with no composor nor npm installed.
Install Patch Tester for Joomla 4 on a non-git Joomla 4 Installation. You can download the zipped nightly here: Joomla! Nightly Builds. The Patch Tester for Joomla 4 is still WIP, download link will follow soon.
In your Joomla instance, go to the Patch Tester and fetch the list of open PRs from GitHub.
Click on apply on this Pull Request's entry.
On your file system, check all files modified by this pull request (see changed files)
Note the CI server address as
SERVER-ADDR.Note the Pull ID of this Pull Request as
PULL-ID.Download
https://<SERVER-ADDR>/<PULL-ID>/build.zipDownload
https://<SERVER-ADDR>/<PULL-ID>/deleted-files.logExpected result
All the file changes are applied on the instance.
You can still use your Joomla instance, nothing is broken.
build.zipdeleted-files.logDocumentation Changes Required
Patch Tester documentation should be updated.