> On Dec. 16, 2015, 11:29 a.m., John Sirois wrote: > > src/test/java/org/apache/aurora/scheduler/http/AbstractJettyTest.java, line > > 148 > > <https://reviews.apache.org/r/41453/diff/1/?file=1166660#file1166660line148> > > > > It looks like this can be an NPE without advanced knowledge of how the > > base class is used in practice, complicated by the fact the field is set > > inside a try - seems the setting could be done outside the try, but ...
Yeah, this patch was a bit scatterbrained. Thanks for pushing back. > On Dec. 16, 2015, 11:29 a.m., John Sirois wrote: > > src/test/java/org/apache/aurora/scheduler/http/AbstractJettyTest.java, line > > 156 > > <https://reviews.apache.org/r/41453/diff/1/?file=1166660#file1166660line156> > > > > The Optional indirection (+ volatile bit) seems a bit odd - the > > startupService is never absent. How about just keep the variable local and > > non-Optional and on the next line add the stopAsync as a tear down? Totally forgot about teardown, that's a much better approach. - Bill ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41453/#review110725 ----------------------------------------------------------- On Dec. 16, 2015, 10:19 a.m., Bill Farner wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41453/ > ----------------------------------------------------------- > > (Updated Dec. 16, 2015, 10:19 a.m.) > > > Review request for Aurora and Joshua Cohen. > > > Repository: aurora > > > Description > ------- > > HTTP server cleanup - shut down the server after unit tests, remove jetty bug > workaround. > > > Diffs > ----- > > src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java > 838bfc9ed6242e777d81a17b337f342b7bea72ec > src/test/java/org/apache/aurora/scheduler/http/AbstractJettyTest.java > 5768481005ef505d6c397101d8cc005af9d6815f > > Diff: https://reviews.apache.org/r/41453/diff/ > > > Testing > ------- > > > Thanks, > > Bill Farner > >
