geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Robert Houghton <rhough...@pivotal.io>
Subject Re: [DISCUSS] reduce PR checks to JDK11 only
Date Mon, 20 May 2019 15:50:43 GMT
The PR pipeline should be a timely test, but also sane and helpful. Maybe
making the JDK8 Unit tests (but not the integration, etc) part of PR is a
good compromise in that sense.

On Sun, May 19, 2019 at 2:20 AM Owen Nichols <onichols@pivotal.io> wrote:

> Correct, analyze-serializables test is currently skipped under JDK11.
> Ideally it would be re-written to test what is actually serialized, not the
> flawed assumption that no change in compiled bytecode size means nothing
> changed...
>
> This also brings up a good question worth discussing: is the goal of PR
> checks to catch all of the same issues as the main pipeline, or is it ever
> ok to run some tests only in the main pipeline (for example, PR checks have
> never included Windows tests).
>
> So which is the preferred course of action?
> 1) restore ALL JDK8 tests to PR checks (i.e. revert PR 3598)
> 2) restore just the one JDK8 test job that includes analyze-serializables
> to the PR checks
> 3) create a tailored subset of JDK8-specific tests to include in PR checks
> 4) just let the main pipeline catch analyze-serializables failures?
> 5) bite the bullet and fix analyze-serializables to test serialization in
> a more meaningful way that works on both JDK8 and 11
>
> -Owen
>
> > On May 17, 2019, at 1:21 PM, Darrel Schneider <dschneider@pivotal.io>
> wrote:
> >
> > One problem with doing this, I think, is that we have some tests that are
> > disabled unless run on 8. They are the analyze serializables tests. I'm
> not
> > sure of the details but I think the "golden" checksums these tests
> compare
> > to were generated from the byte codes from the jdk 8 class files. The
> byte
> > codes are different for jdk 11. So by pull requests runs only happening
> on
> > jdk 11 we will lose coverage. These tests catch if changed the
> serializable
> > format of classes. I think if the "golden" checksums were regenerated
> with
> > jdk 11 then these tests could be enabled when run on jdk 11. Others on
> the
> > dev list may have more of the details.
> >
> > On Thu, May 16, 2019 at 5:31 PM Owen Nichols <onichols@pivotal.io>
> wrote:
> >
> >> I’ve created a PR for this, please give it a +1:
> >> https://github.com/apache/geode/pull/3598
> >>
> >>> On May 16, 2019, at 11:12 AM, Anilkumar Gingade <agingade@pivotal.io>
> >> wrote:
> >>>
> >>> Make sense to me...Looking at the probability of breaking specific to,
> >> jdk8
> >>> and jdk11 through a commit.
> >>>
> >>>
> >>> On Wed, May 15, 2019 at 6:09 PM Owen Nichols <onichols@pivotal.io>
> >> wrote:
> >>>
> >>>> Currently every PR commit triggers both JDK8 and JDK11 versions of
> each
> >>>> test job.  I propose that we can eliminate the JDK8 version of each
> >> check.
> >>>> In the extremely rare case where a code change breaks on Java 8 but
> >> works
> >>>> fine on Java 11, it would still be caught by the main pipeline (just
> as
> >>>> Windows failures are caught only in the main pipeline).
> >>>>
> >>>> The only tangible effect today of running both JDK8 and JDK11 tests
in
> >> PR
> >>>> pipeline is twice the chance to encounter possible flaky failures
> >> (usually
> >>>> unrelated to the commit itself).
> >>>>
> >>>>
> >>>>
> >>
> >>
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message