-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19391/#review37726
-----------------------------------------------------------

Ship it!


Ship It!

- Maxim Khutornenko


On March 19, 2014, 7:31 a.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19391/
> -----------------------------------------------------------
> 
> (Updated March 19, 2014, 7:31 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> I still need to do research on why some of these changes were needed, but 
> this makes our build green on JDK 8 (minus disabling jacoco, which was fixed 
> for JDK 8 today, just waiting for the gradle plugin to update).
> 
> StorageTestUtil.java:99: error: unreported exception E; must be caught or 
> declared to be thrown
>     return expect(storage.write(capture(work))).andAnswer(new IAnswer<T>() {
>                                ^
> It seems that the type on the Capture and the type constraint on the 
> signature of the storage methods aren't treated as expected.  I have a sense 
> this one is a bug in the type system.
> 
> 
> SchedulingFilterImplTest.java:133: error: incompatible types: <anonymous 
> IAnswer<Object>> cannot be converted to IAnswer<? extends CAP#1>
>         .andAnswer(new IAnswer<Object>() {
>                    ^
>   where CAP#1 is a fresh type-variable:
>     CAP#1 extends Object from capture of ?
> 
> RecoveryTest.java:102: error: method write in interface Storage cannot be 
> applied to given types;
>     expect(primaryStorage.write(capture(transaction))).andReturn(null);
>                          ^
>   required: MutateWork<T#1,E>
>   found: MutateWork<CAP#1,CAP#2>
>   reason: inference variable T#2 has incompatible bounds
>     equality constraints: MutateWork<?,?>
>     upper bounds: MutateWork<CAP#3,E>,Object
>   where T#1,E,T#2 are type-variables:
>     T#1 extends Object declared in method <T#1,E>write(MutateWork<T#1,E>)
>     E extends Exception declared in method <T#1,E>write(MutateWork<T#1,E>)
>     T#2 extends Object declared in method <T#2>capture(Capture<T#2>)
>   where CAP#1,CAP#2,CAP#3 are fresh type-variables:
>     CAP#1 extends Object from capture of ?
>     CAP#2 extends Exception from capture of ?
>     CAP#3 extends Object from capture of ?
> 
> I'm not sure the reasoning here, but these seem semi-legitimate.  Looks like 
> the typing of wildcards was made more strict, and these calls lacked 
> sufficient type information to turn the wildcards into _something_.
> 
> 
> GuiceUtils.java:92: warning: [rawtypes] found raw type: Class
>     final LoadingCache<Method, Pair<String, Class[]>> cache = 
> CacheBuilder.newBuilder()
>                                             ^
>   missing type arguments for generic class Class<T>
>   where T is a type-variable:
>     T extends Object declared in class Class
> 
> This is legit, and something i had omitted since adding the wildcard type 
> parameter didn't work with the checkstyle rule.  I chose to drop the 
> checkstyle rule.
> 
> 
> LogManagerTest.java had a failing unit test.  The test relied on consistent 
> ordering of elements within a HashSet, which is bound to be flaky anyway.
> 
> 
> Diffs
> -----
> 
>   config/checkstyle/checkstyle.xml 99f48b2792eb8a7619423088a3ab40fb3a3df772 
>   src/main/java/org/apache/aurora/GuiceUtils.java 
> 9342d4fe7638cbf16233e6f1267b94cd910b6c84 
>   
> src/main/java/org/apache/aurora/scheduler/storage/testing/StorageTestUtil.java
>  543536384bf9630fb93e7db5ba7286c117638199 
>   
> src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
>  d2e1166860a4c3241b9f7f3e1fd420fb42133d45 
>   src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java 
> b85e270d748601c0e979581cde4b1ce273cb2ee2 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogManagerTest.java 
> 77c292d7fc985f385f36979fa90a832de565fef7 
> 
> Diff: https://reviews.apache.org/r/19391/diff/
> 
> 
> Testing
> -------
> 
> Installed JDK 8, temporarily changed build.gradle (see below).  Build is 
> green (there are a few compiler warnings to fix
> 
> diff --git a/build.gradle b/build.gradle
> index f38888b..8c58004 100644
> --- a/build.gradle
> +++ b/build.gradle
> @@ -18,7 +18,6 @@ apply plugin: 'about'
>  apply plugin: 'application'
>  apply plugin: 'checkstyle'
>  apply plugin: 'idea'
> -apply plugin: 'jacoco'
>  apply plugin: 'java'
>  apply plugin: 'license'
>  apply plugin: 'maven-publish'
> @@ -42,8 +41,8 @@ def generatedJavaDir = "$generatedDir/gen-java"
>  def generatedJSDir = "$generatedDir/gen-js"
> 
>  compileJava {
> -  sourceCompatibility = 1.7
> -  targetCompatibility = 1.7
> +  sourceCompatibility = 1.8
> +  targetCompatibility = 1.8
>  }
> 
>  tasks.matching { it instanceof Compile && it.getName() != 
> 'compileGeneratedJava' }.all {
> @@ -347,17 +346,6 @@ license {
>    ext.year = Calendar.getInstance().get(Calendar.YEAR)
>  }
> 
> -jacocoTestReport {
> -  group = "Reporting"
> -  description = "Generate Jacoco coverage reports after running tests."
> -  additionalSourceDirs = files(sourceSets.main.allJava.srcDirs)
> -  doLast {
> -    println "Coverage report generated: 
> file:///$buildDir/reports/jacoco/test/html/index.html"
> -  }
> -}
> -
> -test.finalizedBy jacocoTestReport
> -
>  task FlagSchemaChanges(type: Test) {
>    exec {
>      executable = 'bash'
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Reply via email to