> On Nov. 4, 2014, 11:28 p.m., Bill Farner wrote: > > I'm not a fan of the refactor. For one, static analysis tools will no > > longer catch missing case coverage, and we have to expose the internal > > detail of this map. Instead, the unit test should self-check to make sure > > that it is exercising all transaction operation types. > > Maxim Khutornenko wrote: > "For one, static analysis tools will no longer catch missing case > coverage" - it's actually the opposite. There was no previous enforcement but > after this refactoring there must be a test case or the build fails with this: > ``` > Test coverage missing for > org/apache/aurora/scheduler/storage/log/LogStorage$21 > Test coverage missing for > org/apache/aurora/scheduler/storage/log/LogStorage$11 > Test coverage missing for > org/apache/aurora/scheduler/storage/log/LogStorage$10 > Test coverage missing for > org/apache/aurora/scheduler/storage/log/LogStorage$13 > Test coverage missing for > org/apache/aurora/scheduler/storage/log/LogStorage$12 > Test coverage missing for > org/apache/aurora/scheduler/storage/log/LogStorage$15 > Test coverage missing for > org/apache/aurora/scheduler/storage/log/LogStorage$17 > Test coverage missing for > org/apache/aurora/scheduler/storage/log/LogStorage$16 > Test coverage missing for > org/apache/aurora/scheduler/storage/log/LogStorage$18 > Test coverage missing for > org/apache/aurora/scheduler/storage/log/LogStorage$20 > Test coverage missing for > org/apache/aurora/scheduler/storage/log/LogStorage$2 > Test coverage missing for > org/apache/aurora/scheduler/storage/log/LogStorage$3 > Test coverage missing for > org/apache/aurora/scheduler/storage/log/LogStorage$4 > Test coverage missing for > org/apache/aurora/scheduler/storage/log/LogStorage$5 > Test coverage missing for > org/apache/aurora/scheduler/storage/log/LogStorage$9 > ``` > > Bill Farner wrote: > Touche! That's even better. However, i still would rather avoid leaking > the internal Map if we can help it.
I am not sure how else we can fully validate the expected coverage without exposing the internal defs. Any suggestions? - Maxim ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27598/#review59868 ----------------------------------------------------------- On Nov. 4, 2014, 11:10 p.m., Maxim Khutornenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27598/ > ----------------------------------------------------------- > > (Updated Nov. 4, 2014, 11:10 p.m.) > > > Review request for Aurora, Kevin Sweeney and Bill Farner. > > > Bugs: AURORA-912 > https://issues.apache.org/jira/browse/AURORA-912 > > > Repository: aurora > > > Description > ------- > > Fixing the log replay for the job update history pruner. > > Also, refactored LogStorage replay routine to fully test all > LogEntry/LogEntry.TRANSACTION handlers. > > > Diffs > ----- > > src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java > cbab75964052a950e1b868b3a53eb15fadb31cb7 > src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java > 8eb5c3f7c542206066b39a09911c7df01a43bee7 > > Diff: https://reviews.apache.org/r/27598/diff/ > > > Testing > ------- > > ./gradlew -Pq build > > > Thanks, > > Maxim Khutornenko > >