Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [jgit-dev] GerritForge's JGit E2E tests with Gerrit in 2022

It certainly will be worrying if you declare JGit 5.13.0 end of life.  The Jenkins project uses it in the git client plugin.

On Thu, Jan 20, 2022 at 1:25 PM Luca Milanesio <luca.milanesio@xxxxxxxxx> wrote:


On 20 Jan 2022, at 20:05, Matthias Sohn <matthias.sohn@xxxxxxxxx> wrote:

On Thu, Jan 20, 2022 at 6:22 PM Luca Milanesio <luca.milanesio@xxxxxxxxx> wrote:


On 20 Jan 2022, at 13:09, Matthias Sohn <matthias.sohn@xxxxxxxxx> wrote:

On Thu, Jan 20, 2022 at 12:46 PM Luca Milanesio <luca.milanesio@xxxxxxxxx> wrote:
Hi Matthias, thanks for sharing your feedback.
See my comments below.

On 20 Jan 2022, at 00:05, Matthias Sohn <matthias.sohn@xxxxxxxxx> wrote:

On Tue, Jan 18, 2022 at 4:37 PM Luca Milanesio <luca.milanesio@xxxxxxxxx> wrote:
Dear Gerrit and JGit developers’ community,
I would like to share with you the GerritForge’s plans for adding more E2E tests to JGit changes in 2022 [1].

Opinions, suggestions, objects, any sort of feedback is more than welcome :-)

Luca.

[1] https://gitenterprise.me/2022/01/18/new-year-new-jgit-contributions-coming/

I would prefer to see your proposals posted on this list instead of your website since it would be easier
to respond and follow the conversation.

Sure, all proposals for the JGit project and associated discussion will happen here.
The [1] above is a GerritForge’s initiative announcement, like the JGit E2E tests with Gerrit we introduced a long ago and published in the past [6].

I think the main reason why some contributions to jgit stay in review for a long time is a lack of active reviewers.

True, I agree with your analysis. However, some of the reviewers and committers who wrote a large part of the code-base are inactive and won't be back to help us out.
The only solution is to increase the volume of reviews and get more changes merged. We do need more people with more knowledge and experience with the code.

yes, and to increase the volume of reviews we need to focus on less branches and find more reviewers

Understood and agreed: we will focus on *master* branch only with new changes and reviews.
We will post fixes on stable-6.0 if we find bugs.

 
Currently, I can only observe the numbers that show a yearly decline in the number of merged changes. Also, the number of active committers is in evident decline.
GerritForge wants to help here, provide more testing, more input, more reviews, more ideas, more changes, and make things better for everyone.

This won't improve by adding yet another branch in a fork. I think the existing jgit master branch is the
development branch.

The focus is on making JGit + Gerrit tests work E2E, not creating any fork.

The workflow of using a “runway” of changes all cherry-picked (or rebased) and getting extra integration tests *before* merging them is pretty common.
Also, Zuul implements *that* workflow (see [7] for an overview of how the process works).

We spent quite some time in the past on reviewing and testing patches for older jgit releases and
merging them up to master since Gerrit was reluctant to update older Gerrit maintenance branches to
use the latest JGit maintenance branch.  This added overhead and ate some of the limited capacity we have. 

Hence I propose we stop this to better use our limited resources by
  • limiting development of new features to the master branch
  • limiting maintenance of releases to the latest release (currently 6.0)
  • maybe as an exception to this rule allow fixes for the last minor release of the last major release (currently 5.13).
    Some users asked for this since they still depend on Java 8 which is no longer supported since jgit 6.0
  • declaring EOL on all older releases
I fully agree with all of the above: I believe it deserves a separate discussion thread because of its importance and increased visibility and discoverability. This topic is about testing JGit and Gerrit together on master branch.

Gerrit actively maintains its last 3 releases (currently 3.3, 3.4, 3.5). Last week the Gerrit Engineering Steering Committee
defined a new policy for updating JGit [1] on Gerrit maintenance branches supporting this proposal to reduce the number
of active JGit maintenance branches. 

Yes, I recall the discussion, and there is a global consensus on that.
I believe that having E2E tests of JGit + Gerrit executed on the “changes on the runway” to be merged would also help the Gerrit side update the submodule pointer to JGit master soon as possible.

P.S. Currently, JGit master and Gerrit master don’t even build together until Gerrit change [5] is merged.

that's because Gerrit doesn't update the jgit version it uses regularly

Yep, we are working to fix that and keep them working in sync.

I appreciate the proposal to run more tests like Gerrit integration tests against open jgit changes.

+1

I don't understand why another branch is needed for implementing that.
Instead these tests should be run
on the open changes.

You do need a "stream of changes together" (to highlight that the focus is NOT the branch) because when you run the integration tests, you need to see and check things working together, not in isolation.

First of all, each change in review needs to work in isolation. Next change series typically used to implement larger features
need to work as a whole and then we can look at testing the combination of multiple new features.

+1

 
Let me give you a simple example:
  • Change Ifoo removes a foo() method in JGit that is not used by anyone
  • Change Ibar introduces a new bar() method based on the foo() method
  • You verify Change Ifoo and Ibar in isolation (NOT streamlined) and they both get the Verified +1 and merged. 
Verifying all individual changes in isolations won't tell you if they work together.

With the "stream of changes", you will do instead:
  • Change Ifoo removes a foo() method in JGit that no one uses.

if that's a public API this should only occur in a major release, if it's not a public API, it's a problem of the consumers who have decided to use non-API.
We do compare the current API against the last release using Eclipse API tools and using japicmp in the maven build.
We could configure the latter to fail the build if such a breaking change occurs in a minor or micro version.

In theory, yes. In practice, it is always best to test the interaction between components E2E.
Sometimes there are subtle differences that are NOT visible in the syntax of the public API but result in significant regressions.

 
  •  Change Ibar introduces a new bar() method based on the foo() method.
  • Change Ifoo and Ibar are put (cherry-picked or rebased) to the "validation stream" and tested together. The build breaks, and none of the two changes are validated.
The above example is very simplistic. The problem is a lot more complex than that; however, it shows that creating a "stream of changes" for validation is key when you want to integrate and validate multiple components.

There is also an issue with execution times and tradeoffs with the speed of validating incoming changes.
Running E2E tests takes a long time (hours) and is very expensive (hundreds of $ of infrastructure time): you want to limit that to ONLY those changes that:
- Have passed the normal verification process (e.g. Eclipse's CI)
- Have been reviewed and have no vetos from being approved

that's the current bottleneck

Not sure I understand: we don’t run E2E tests at the moment with JGit changes validation: how can that be the current bottleneck? Can you rephrase it?

I meant that ensuring changes have been reviewed and have no veto is the current bottleneck since we need more active reviewers 
The two aspects depicted above represent the reason why the trigger for running the E2E tests *cannot* be applied for all open changes targeting master.

JGit participates in the Eclipse IDE's simultaneous release [8] where integration happens on a milestone basis,
each release has 3 milestones 3 weeks apart and concludes in 2 release candidates, the last one is typically the next minor jgit release.
Obviously, there IDE scenarios are in the focus.

I know, and Gerrit has a different release cycle and, often, uses an older version of JGit.
We are aiming to fix that.

Running more tests can help to find more problems and prevent regressions but
doesn't fix the problem that we need more active reviewers.

Sure, that is crystal clear. Having more E2E tests of JGit with Gerrit will increase the confidence in merging changes that have been stuck in review for a very long time.
We address the confidence in merging changes where current active reviewers and commits have a lack of knowledge of the underlying code. 

IMHO projects should not be stuck because the original author of the code isn’t around anymore to maintain it.
Open-Source is great because allows the code initially written by one author to be reviewed, modified, extended by the whole community.

The lack of knowledge should never impede contributions, reviews or approvals.
If I am not confident about one change, I’ll write more tests to verify its correctness.
If I don’t understand some parts of the source code, I’ll spend more time reading it more carefully.

I think that's what is happening, though since it needs more time to do this on code you are less familiar with
this slows down progress.

Cool, I’ll resume some old changes that got left behind because of lack of knowledge: let get them merged !
+1

And JGit not only gets contributions used by Gerrit on the server side but also for
client scenarios like the large change series for adding support for external merge and diff tools.

Is there a list of “well known users” of JGit?
It would be useful to have visibility on those, so that we can be more aware of those scenarios when we make and review changes in JGit.

It seems there are a lot of them. The main jgit library org.eclipse.jgit has more than 4mio downloads per month from maven central:
 
<Screenshot 2022-01-20 at 20.13.07.png>

Apparently there are a lot of consumers of some old releases.

mvnrepository.com lists 1369 artefacts using org.eclipse.jgit:org.eclipse.jgit:

Wow, that is impressive and worrying from some points of view: maybe something to take into account if we decide to declare all of them EOL?

Luca.


 
Our efforts in improving the E2E testing of JGit with Gerrit is directed at *improving* the confidence of current and new reviewers and contributors.
I hope that the JGit community would appreciate our initiative and help with our endeavour.


The JGit CI [2] uses Eclipse CBI [3] to run builds and tests on Jenkins running in Kubernetes.
I guess the proposed tests could also be run there. If we need more resources there we can add some more.

The tests are E2E with a real Gerrit setup, including:
- load-balancers
- LDAP authentication
- Shared filesystem on NFS
- multiple Gerrit primaries

This Gerrit input qualification then better happens on Gerrit's own CI where the necessary infrastructure is already available.

Sure, and it *does* already happen on the Gerrit-CI (see [9]). We are just reusing what we have already in place.

 
You know better than me that only executing in a production environment we could discover some very nasty JGit bugs, and we (actually you) managed to fix them before the Gerrit release.
We want to facilitate and help with the process and adding more testing.

+1
As proposed by David Ostrovsky we should also aim at verifying the bazel build and tests in JGit CI.
Unfortunately it can't fully replace the Maven build since there is no support in bazel for building OSGi bundles,
Eclipse features and p2 repository which is required for installing it in Eclipse IDE.

Sure, that is something we should do regardless.

If someone has time to help improving JGit CI we'd appreciate that. Thomas Wolf implemented a pipeline
library for the egit builds [4], but we didn't find time yet to do something similar for the jgit builds.

I’d be happy to help with that, as I already do it for the Gerrit project.
Do you know where the JGit pipeline is stored and maintained?

Currently the JGit build is manually configured in Jenkins jobs. 

Sure, just let me know who I need to get in touch with, so that I can help and support him.
I good idea would be moving to a JCasC-like definition of the CI for JGit? (See [10])

I am looking forward to your and your colleagues' contributions. If they are coming as you said we may have
some new jgit committers soon. If possible please also plan time to help with reviewing changes from other
contributors.

Sounds good, looking forward to it !
Thanks again for taking the time to provide your valuable feedback and reading about initiative.

Luca.


[5] https://gerrit-review.googlesource.com/c/gerrit/+/327079


[9] https://gerrit-ci.gerritforge.com/job/gatling-gerrit-test/



-Matthias

_______________________________________________
jgit-dev mailing list
jgit-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/jgit-dev

Back to the top