? ? Success

User tests: Successful: Unsuccessful:

avatar eXsiLe95
eXsiLe95
10 Sep 2019

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

  • Add 1 step in Drone CI
    • First the reference branch (4.0-dev) is updated (if necassary)
    • Next, the difference between the active branch and the reference branch is compared. This diff is archived into a zip file and uploaded to the CI server where the patch tester can download and apply the patch.

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:

  • 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.zip
  • Download https://<SERVER-ADDR>/<PULL-ID>/deleted-files.log

Expected result

  • build.zip
    • The file should contain all new/modified files of this pull request.
  • deleted-files.log
    • The file should contain all deleted files of this pull request.

When 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.zip

  • Download https://<SERVER-ADDR>/<PULL-ID>/deleted-files.log

Expected result

  • All the file changes are applied on the instance.

  • You can still use your Joomla instance, nothing is broken.

  • build.zip

    • The file should contain all new/modified files of this pull request.
  • deleted-files.log

    • The file should contain all deleted files of this pull request.

Documentation Changes Required

Patch Tester documentation should be updated.

avatar eXsiLe95 eXsiLe95 - open - 10 Sep 2019
avatar eXsiLe95 eXsiLe95 - change - 10 Sep 2019
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 10 Sep 2019
Category Unit Tests
avatar eXsiLe95 eXsiLe95 - change - 10 Sep 2019
Labels Added: ? ?
avatar franz-wohlkoenig franz-wohlkoenig - change - 10 Sep 2019
Title
Patch tester for joomla4
[4.0] Patch tester for joomla4
avatar franz-wohlkoenig franz-wohlkoenig - edited - 10 Sep 2019
avatar eXsiLe95
eXsiLe95 - comment - 10 Sep 2019

@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.

avatar eXsiLe95 eXsiLe95 - change - 10 Sep 2019
Title
[4.0] Patch tester for joomla4
[4.0] Patch tester enhancement for joomla4 (pre-compiled diff)
avatar eXsiLe95 eXsiLe95 - edited - 10 Sep 2019
avatar eXsiLe95 eXsiLe95 - change - 10 Sep 2019
The description was changed
avatar eXsiLe95 eXsiLe95 - edited - 10 Sep 2019
avatar eXsiLe95 eXsiLe95 - change - 10 Sep 2019
The description was changed
avatar eXsiLe95 eXsiLe95 - edited - 10 Sep 2019
avatar N6REJ
N6REJ - comment - 11 Sep 2019

@eXsiLe95

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.

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.

avatar brianteeman
brianteeman - comment - 11 Sep 2019

@N6REJ thats the entire point of this code. So that you can test patches as easily in J4 as you could in J3

avatar Hackwar
Hackwar - comment - 11 Sep 2019

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.

avatar alikon
alikon - comment - 11 Sep 2019

maybe a silly/naive question....
if i understand correctly this proposal, (forgive me in advance if i misunderstand something or all)
this will work only on j4
if my guess is true
we need 2 different com_patchtester
1 for 3.x
and
1 for 4.x
correct ?

avatar eXsiLe95
eXsiLe95 - comment - 11 Sep 2019

we need 2 different com_patchtester
1 for 3.x
and
1 for 4.x
correct ?

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.

avatar N6REJ
N6REJ - comment - 11 Sep 2019

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.
i'm not a pro coder like @mbabker but even I know that the installer can automagically figure out if its installing on j3 or j4 ergo it can internally make any adjustments needed itself!
The fact that your no longer worried about whether it will work for 3.x or not is an issue in itself.

In the global configuration, you can choose whether to use the common version or get the diff from CI

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.
The integration that was done with patch tester & issue tracker & git was fantastic. A complete game changer. It sounds like you now want to change all that.

avatar N6REJ
N6REJ - comment - 11 Sep 2019

@eXsiLe95 Am I understanding correctly that this "new" patch tester will REQUIRE docker?

avatar Hackwar
Hackwar - comment - 12 Sep 2019

@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.
You are unhappy with the Patchtester being extended and used for more than the pure concept of downloading files from Github. If you really want to stick to that "pure" approach, then you might want to switch to another CMS, because Joomla does a lot more than just handle content. It also handles users, session data, etc.
Anyway, I'm currently considering forking the Patchtester as com_changetester, so that we don't have these constant discussions if it is now a patch that we are testing or a diff or simply replacing files. Good god.

@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.

avatar Hackwar
Hackwar - comment - 12 Sep 2019

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.

avatar Hackwar
Hackwar - comment - 12 Sep 2019

@eXsiLe95 and make sure that composer and npm are run for the reference. ?

avatar N6REJ
N6REJ - comment - 12 Sep 2019

@N6REJ No, this does not require Docker. Why should it?

This file is a docker file is it not? So, if its not requiring docker then I'm totally confused.

avatar eXsiLe95
eXsiLe95 - comment - 12 Sep 2019

@N6REJ No, this does not require Docker. Why should it?

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!

avatar Hackwar
Hackwar - comment - 12 Sep 2019

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.

avatar N6REJ
N6REJ - comment - 12 Sep 2019

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.

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.

avatar HLeithner HLeithner - change - 13 Sep 2019
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2019-09-13 17:49:33
Closed_By HLeithner
avatar HLeithner HLeithner - close - 13 Sep 2019
avatar HLeithner HLeithner - merge - 13 Sep 2019
avatar HLeithner
HLeithner - comment - 13 Sep 2019

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.

Add a Comment

Login with GitHub to post a comment