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