[dubbo-1521]add java 9 in travis yml #1575
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1575 +/- ##
============================================
+ Coverage 32.17% 34.43% +2.26%
- Complexity 0 3669 +3669
============================================
Files 682 618 -64
Lines 32972 30823 -2149
Branches 6597 5445 -1152
============================================
+ Hits 10608 10615 +7
+ Misses 20468 18378 -2090
+ Partials 1896 1830 -66
Continue to review full report at Codecov.
|
| </configuration> | ||
| <groupId>org.jacoco</groupId> | ||
| <artifactId>jacoco-maven-plugin</artifactId> | ||
| <version>${maven_jacoco_version}</version> |
There was a problem hiding this comment.
jacoco instead of cobertura？**
There was a problem hiding this comment.
@kimmking yes, this pr is using jacoco.
cobertura is no longer active since 2015. There is no support for java9 and higher version.
so in order to support java9 and even java10, we must try another tool to replace cobertura
jacoco is a good choice here:
- jacoco use agent to capture test coverage, it's more quick
- jacoco has a active team and also it support java9 and java10
- jacoco has lots of users in community. for example, gradle has built-in support for jacoco
|
Have you tested when the program error or the test case doesn't pass, will the CI process terminate? @htynkn |
|
@lovepoem please refer this build: https://travis-ci.org/diamondblack/incubator-dubbo |
|
Look good to me |
| @@ -910,6 +910,9 @@ private static Object getEmptyObject(Class<?> returnType, Map<Class<?>, Object> | |||
| while (cls != null && cls != Object.class) { | |||
| Field[] fields = cls.getDeclaredFields(); | |||
| for (Field field : fields) { | |||
| if (field.isSynthetic()) { | |||
There was a problem hiding this comment.
is it necessary to include this change?
| @@ -910,6 +910,9 @@ private static Object getEmptyObject(Class<?> returnType, Map<Class<?>, Object> | |||
| while (cls != null && cls != Object.class) { | |||
| Field[] fields = cls.getDeclaredFields(); | |||
| for (Field field : fields) { | |||
| if (field.isSynthetic()) { | |||
There was a problem hiding this comment.
What's the purpose to add this filter?
There was a problem hiding this comment.
@lovepoem @beiwei30
please refer to jacoco faq http://www.jacoco.org/jacoco/trunk/doc/faq.html
We need this change to skip $jacocoData, otherwise test will fail.
Also skip synthetic field in reflection is also a good practice.
Refer to those two links:
- https://docs.oracle.com/javase/tutorial/reflect/member/fieldModifiers.html
- https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.7.8
According to the JVM Spec: "A class member that does not appear in the source code must be marked using a Synthetic attribute." Also, "The Synthetic attribute was introduced in JDK release 1.1 to support nested classes and interfaces."
There was a problem hiding this comment.
class is also added synthetic checking?
What is the purpose of the change
add java9 in ci
Brief changelog
use jacoco
fix reflection issue
Verifying this change
ci pass
coverage in codecov.io
close #1521