I that the serialization test was only an issue when compiled with JDK11? We don’t compile with JDK11, we compile with JDK8 and run under JDK11.
> On May 20, 2019, at 8:50 AM, Robert Houghton <rhough...@pivotal.io> wrote: > > 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 <onich...@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 <dschnei...@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 <onich...@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 <aging...@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 <onich...@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). >>>>>> >>>>>> >>>>>> >>>> >>>> >> >>
