Hey Nick, Thanks for bringing this up. I believe these Java tests are running in the sbt build right now, the issue is that this particular bug was flagged by the triggering of a runtime Java "assert" (not a normal Junit test assertion) and those are not enabled in our sbt tests. It would be good to fix it so that assertions run when we do the sbt tests, for some reason I think the sbt tests disable them by default.
I think the original issue is fixed now (that Sean found and reported). It would be good to get assertions running in our tests, but I'm not sure I'd block the release on it. The normal JUnit assertions are running correctly. - Patrick On Tue, Dec 9, 2014 at 3:35 PM, Nicholas Chammas <nicholas.cham...@gmail.com> wrote: > OK. That's concerning. Hopefully that's the only bug we'll dig up once we > run all the Java tests but who knows. > > Patrick, > > Shouldn't this be a release blocking bug for 1.2 (mostly just because it > has already been covered by a unit test)? Well, that, as well as any other > bugs that come up as we run these Java tests. > > Nick > > On Tue Dec 09 2014 at 6:32:53 PM Sean Owen <so...@cloudera.com> wrote: > >> I'm not so sure about SBT, but I'm looking at the output now and do >> not see things like JavaAPISuite being run. I see them compiled. That >> I'm not as sure how to fix. I think I have a solution for Maven on >> SPARK-4159. >> >> On Tue, Dec 9, 2014 at 11:30 PM, Nicholas Chammas >> <nicholas.cham...@gmail.com> wrote: >> > So all this time the tests that Jenkins has been running via Jenkins and >> SBT >> > + ScalaTest... those haven't been running any of the Java unit tests? >> > >> > SPARK-4159 only mentions Maven as a problem, but I'm wondering how these >> > tests got through Jenkins OK. >> > >> > On Tue Dec 09 2014 at 5:34:22 PM Sean Owen <so...@cloudera.com> wrote: >> >> >> >> Yep, will do. The test does catch it -- it's just not being executed. >> >> I think I have a reasonable start on re-enabling surefire + Java tests >> >> for SPARK-4159. >> >> >> >> On Tue, Dec 9, 2014 at 10:30 PM, Aaron Davidson <aa...@databricks.com> >> >> wrote: >> >> > Oops, that does look like a bug. Strange that the >> >> > BlockTransferMessageSuite >> >> > did not catch this. "+1" sounds like the right solution, would you be >> >> > able >> >> > to submit a PR? >> >> > >> >> > On Tue, Dec 9, 2014 at 1:53 PM, Sean Owen <so...@cloudera.com> wrote: >> >> >> >> >> >> >> >> >> >> >> >> https://github.com/apache/spark/blob/master/network/ >> shuffle/src/main/java/org/apache/spark/network/shuffle/ >> protocol/BlockTransferMessage.java#L70 >> >> >> >> >> >> public byte[] toByteArray() { >> >> >> ByteBuf buf = Unpooled.buffer(encodedLength()); >> >> >> buf.writeByte(type().id); >> >> >> encode(buf); >> >> >> assert buf.writableBytes() == 0 : "Writable bytes remain: " + >> >> >> buf.writableBytes(); >> >> >> return buf.array(); >> >> >> } >> >> >> >> >> >> Running the Java tests at last might have turned up a little bug >> here, >> >> >> but wanted to check. This makes a buffer to hold enough bytes to >> >> >> encode the message. But it writes 1 byte, plus the message. This >> makes >> >> >> the buffer expand, and then does have nonzero capacity afterwards, so >> >> >> the assert fails. >> >> >> >> >> >> So just needs a "+ 1" in the size? >> >> > >> >> > >> >> >> >> --------------------------------------------------------------------- >> >> To unsubscribe, e-mail: dev-unsubscr...@spark.apache.org >> >> For additional commands, e-mail: dev-h...@spark.apache.org >> >> >> > >> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@spark.apache.org For additional commands, e-mail: dev-h...@spark.apache.org
