> On July 6, 2015, 3:02 a.m., Benjamin Hindman wrote: > >
Hey Ben, Great questions in the latter half of your review. It turns out that the original test was malformed. It happened to pass because there is also a help process that responds to the `/help` endpoint. The original intent of the test, however, was to ensure that the newly added `/help` endpoint would work. That newly added enpoint was actually never being hit. The name is being passed through to the HTTPProcess constructor explicitly to make the url at which the test endpoint is being hit stable. If the default `Process` constructor is used, then we end up using the auto-incrementing ids which can be hard to deterministically hit. I agree that we should stick with a single endpoint as you suggested; however, we need to change the name to something unique to avoid this test from passing accidentally when the intended path is broken. We definitely need a comment as to why we're explicitly specifying the name of the process (as described above). I have seen a couple of people bang their heads against this, so let's add cleared documentation to the headers / doxygen as well! - Joris ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35755/#review90423 ----------------------------------------------------------- On July 6, 2015, 4:03 p.m., Artem Harutyunyan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35755/ > ----------------------------------------------------------- > > (Updated July 6, 2015, 4:03 p.m.) > > > Review request for mesos and Joris Van Remoortere. > > > Bugs: MESOS-2862 > https://issues.apache.org/jira/browse/MESOS-2862 > > > Repository: mesos > > > Description > ------- > > Fixes MESOS-2862 > > > Diffs > ----- > > src/launcher/fetcher.cpp 8aee4901ec1289f43b5fa6b830c3488815ec24cd > src/tests/fetcher_tests.cpp ae10c420f7dddb8650377c91b5343591e8560392 > > Diff: https://reviews.apache.org/r/35755/diff/ > > > Testing > ------- > > - make check > - added a test case for fetcher > > > Thanks, > > Artem Harutyunyan > >
