> On Aug. 19, 2014, 7:58 p.m., Chinmay Soman wrote: > > samza-core/src/test/scala/org/apache/samza/container/TestSamzaContainer.scala, > > line 58 > > <https://reviews.apache.org/r/24863/diff/1/?file=664733#file664733line58> > > > > I guess when the NPE gets fixed, we will have to modify this test. > > > > How about recreating the same problem : No task for a given SSP - to > > make safeMain throw an exception ?
I opted to just catch exception. This seems to be the most robust way to handle it. I'm reluctant to mess with environment variables in a unit test since it is not thread safe, and could get leaked beyond the one test. I'm also reluctant to make the main method further injectible since the code is barely readable as it is. I think we really need to either expand, or have a similar class as, the CommandLine.scala class, which lets us properly handle this kind of stuff in a unit-testable/mockable way. Both SamzaAppMaster and SamzaContainer could benefit from a bit of refactoring. I'll open a separate ticket for that. - Chris ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24863/#review51026 ----------------------------------------------------------- On Aug. 19, 2014, 8:50 p.m., Chris Riccomini wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/24863/ > ----------------------------------------------------------- > > (Updated Aug. 19, 2014, 8:50 p.m.) > > > Review request for samza. > > > Bugs: SAMZA-382 > https://issues.apache.org/jira/browse/SAMZA-382 > > > Repository: samza > > > Description > ------- > > switching to catch any exception > > > Revert "do environment variable validation in SamzaContainer. clean up JMX > test in SamzaContainer to use container name exception." > > This reverts commit efe56adbf5ac9327f8781da4186f8bb438bd5eb8. > > do environment variable validation in SamzaContainer. clean up JMX test in > SamzaContainer to use container name exception. > > > move main method into a separate method to allow a mock-able JmxServer. write > a test to validate that the jmx server is not properly called on certain > exceptions. fix by moving the try block to include all logic in the main > method. > > > fix random typo in comments > > > Diffs > ----- > > samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala > d574ac413c0ec81e12eb44b2d0cc0d9843aad434 > > samza-core/src/test/scala/org/apache/samza/container/TestSamzaContainer.scala > e3c7fe3e2d329b0767eb439144b1ba419848bb96 > > Diff: https://reviews.apache.org/r/24863/diff/ > > > Testing > ------- > > > Thanks, > > Chris Riccomini > >
