On Thu, Feb 10, 2022 at 4:25 PM Rainer Jung <rainer.j...@kippdata.de> wrote:
>
> Am 10.02.2022 um 14:41 schrieb Rémy Maucherat:
> > On Thu, Feb 10, 2022 at 2:11 PM Mark Thomas <ma...@apache.org> wrote:
> >>
> >> On 09/02/2022 14:04, Rainer Jung wrote:
> >>> Hi all,
> >>>
> >>> I wonder how important it is to be able to run the unit tests for TC
> >>> 8.5-10.0 with JVM versions 8 or even 7 (TC 8.5).
> >>>
> >>> The unconditional addition of unit test jvmargs "--add-opens=..." in
> >>> 2b6e19e971a980e38bcd30f05c554d9b798666c0 (TC 10, backported) means, we
> >>> can no longer run the tests with Java 8 or even 7.
> >>>
> >>> If I remove those jvmargs from build.xml, I run into a second problem
> >>> when testing TC 8.5 with Java 7. In
> >>> 014f4746176c7f27cda7c04b2c6036a539c385ff EasyMock was updated from 3.x
> >>> to 4.x, which needs Java 8 to run. Several tests fail with
> >>> UnsupportedClassVersionError in a few EasyMock classes.
> >>>
> >>> I can work around all of these myself for running the tests here, but I
> >>> wonder, what kind of setup for testing we want to support out-of-the box.
> >>>
> >>> Note that by reintroducing the previous compatibility trick for jvmargs
> >>> (TC 8.5, 9 and 10) and using the old EasyMock branch for TC 8.5 we could
> >>> still build and test with Java 11, but would also allow testing on the
> >>> other target Java versions we support.
> >>>
> >>> WDYT?
> >>
> >> I've been thinking about this since I read this yesterday.
> >>
> >> I like that the move to Java 11 allowed us to reduce the differences
> >> between the build scripts.
> >
> > +1
> >
> >> So the decision is: Is the benefit of being able to test with older Java
> >> versions worth the differences this would re-introduce?
> >>
> >> We don't test the JreCompat code so there aren't any Java version
> >> specific tests. So what do we gain by testing on Java 7 / Java 8? I can
> >> think of a few things:
> >> - that we really have coded to the correct Java version
> >> - that the code runs as expected on that Java version
>
> For example TLS behavior. There are functionalities that have the same
> API in different Java versions, but implementations provide different
> behavior.
>
> >> Both of those things are more testing Java than testing Tomcat but I can
> >> see that there is some benefit - especially if one is concerned about
> >> the possibility of the code behaving differently on Java 7/8 compared to
> >> Java 11.
> >
> > It is a good point that this is about testing Java (the bytecode
> > produced should target Java 8 exclusively - talking about Tomcat 9,
> > for example -, and the behavior of the API should be the same; if
> > either becomes false, then this is actually a Java bug).
>
> Concerning the same API behavior, wouldn't changes in TLS support
> qualify as such a change which is not a bug? Of course, most of the time
> Tomcat would be agnostic in itself to such changes, but I wouldn't be so
> sure that we would not every now and then find a relevant difference via
> the unit tests.
>
> > To be honest, I only partially trust that "release" feature right now.
> > Although it seems to work, it might simply need time to get used to
> > the concept ! I did point out the testsuite problem when this was
> > initially discussed. And I understand the lack of trust if Tomcat is
> > simply not tested on Java 8 (or 7, cough; let's pretend it is really
> > tested ;) ) before release.
> >
> > Instead of completely reverting to the previous properties hack and
> > reintroducing differences between branches, I would simply advocate
> > moving the offending values to hardcoded property values in build.xml
> > (which can then be replaced by Java 8 compatible placeholder values in
> > your own build.properties - same idea than for the dependencies which
> > can be replaced by Java 8 compatible ones).
>
> Sounds like a reasonable compromise. If we would find over time more
> reasons to actually run the tests, would it be easy to define JVM
> version specific build.properties lines on CI nodes? No idea, whether we
> already overwrite build properties on CI.

I added the workaround, in build.properties, you can add:
    opens.javalang=-Dtest10=1
    opens.javaio=-Dtest11=1
    opens.sunrmi=-Dtest12=1
    opens.javautil=-Dtest13=1
    opens.javautilconcurrent=-Dtest14=1

I would build everything first with Java 11 (although the release flag
is ignored if you try on Java 8) with "ant test". Then using "ant
test" again with Java 8 and the properties worked for me.

Rémy

>
> >> I'm not concerned about that but neither am I too concerned about the
> >> re-introduction of a few differences. I guess I am -0 overall provided
> >> that any change does not impact the existing CI systems.
> >
> > Rémy
>
> Thanks and regards!
>
> Rainer
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to