> On Feb. 27, 2015, 5:01 p.m., Bill Farner wrote: > > src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java, > > lines 215-227 > > <https://reviews.apache.org/r/31338/diff/4/?file=876499#file876499line215> > > > > Can this instead be an exact comparison of List<Volume>? Presumably we > > should fail if other volumes show up. > > > > ``` > > assertEquals( > > ImmutableList.of(expected), > > taskInfo.getExecutor().getContainer().getVolumesList()); > > ``` > > Steve Niemitz wrote: > I think checking only this specifically is a more robust test. There are > other things we may want to mount in by default in the future (for example, > we already mount the thermos run directory), and testing for all of them here > would make the test to broad. > > Bill Farner wrote: > Hmm...i see your point, but i don't entirely buy it :-) > > For example, if the code behind this erroneously inserts two > `Protos.Volume` entries for each global mount, the test case should trip.
imo it's more likely that we'll be adding more features around volumes (default or otherwise) than introducing breaking changes to the code that adds the default volumes. Unit tests that are too fragile I think cause more harm than good. - Steve ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31338/#review74507 ----------------------------------------------------------- On Feb. 27, 2015, 11:16 p.m., Steve Niemitz wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31338/ > ----------------------------------------------------------- > > (Updated Feb. 27, 2015, 11:16 p.m.) > > > Review request for Aurora, Jay Buffington and Bill Farner. > > > Bugs: AURORA-1107 > https://issues.apache.org/jira/browse/AURORA-1107 > > > Repository: aurora > > > Description > ------- > > Added a command line flag, -global_container_mounts, to allow mounting paths > from the slaves into the (docker) containers they run. > > This is the first portion of allowing per-job mounts, however, I wanted to > get this out first since more people want it. I'll implement per-job mounts > in a future review. > > > Diffs > ----- > > api/src/main/thrift/org/apache/aurora/gen/api.thrift > 11116f6124a4c844a1abcf07401d80c3e50eb8b4 > config/checkstyle/checkstyle.xml 580d9d3737c0f52da2d33f5b99ef9406b55c5807 > docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f > src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java > bacfbfeb237ecddf82f58679e05be012c5214e61 > src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java > 24b61c1e4f615295acf28d904588e1512972d3f4 > src/main/java/org/apache/aurora/scheduler/app/VolumeParser.java > PRE-CREATION > src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java > baacb71403d55c5b90fc11cb2a23f552a32e8ba5 > src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java > 5340d651b298ec8aa079e73d6d2f652fdf876293 > src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java > 6575b7d420f17ec68d6e2a83e7b380f684577d4f > src/test/java/org/apache/aurora/scheduler/app/VolumeParserTest.java > PRE-CREATION > > src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java > 444d6d3fdaf86eb84612f846eaa326eb75c49898 > src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java > efe62ceb502ead88a2f0cd6d09a76664e465d9bc > > Diff: https://reviews.apache.org/r/31338/diff/ > > > Testing > ------- > > > Thanks, > > Steve Niemitz > >
