Re: Review Request 52639: Added test for `recovered` AgentID and `AGENT_ADDED` after reregister.

[Anand Mazumdar]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52639/#review160203
---



Looks good minus a comment around simplifying the `GetRecoveredAgents` test.


src/tests/api_tests.cpp (lines 1470 - 1472)


As per my review comment earlier, I would like this test to just ensure 
that the `recovered` field is correctly populated in the `GetAgents` response. 
I don't like the idea of this test also checking if the `AGENT_ADDED` event is 
_also_ received. That seems an orthogonal test not _strictly_ related to this 
change. Can you modify this test to just test for the `recovered` field instead?

This would also make it consistent with the other test you added.

Also,
s/checks/verifies
s/reregistered/reregister



src/tests/api_tests.cpp (line 1477)


Nit: Newline before.



src/tests/api_tests.cpp (line 1494)


s/Make sure/Ensure that
s/shown/present



src/tests/api_tests.cpp (line 1495)


s/recovered_agents/GetAgent.recovered_agents`



src/tests/api_tests.cpp (line 1523)


```
// Ensure that the agent is present in `GetAgent.recovered_agents` while 
`GetAgent.agents` is empty.
```



src/tests/api_tests.cpp (lines 1541 - 1581)


Kill this



src/tests/api_tests.cpp (line 1583)


Kill this and instead add a comment before L1587

```
// Start the agent to make it re-register with the master.
```



src/tests/api_tests.cpp (lines 1593 - 1604)


Kill this



src/tests/api_tests.cpp (line 1606)


```
After the agent has successfully re-registered with the master, the 
`recovered_agents` field of `GetAgents` would be empty.
```



src/tests/master_tests.cpp (lines 4032 - 4033)


Can we make this consistent with the comment in `GetRecoveredAgents`



src/tests/master_tests.cpp (line 4034)


Nit: s/StateSlavesEndpointsRecoveredSlaves/RecoveredSlaves



src/tests/master_tests.cpp (line 4038)


Nit: Newline before



src/tests/master_tests.cpp (lines 4064 - 4065)


Modify this comment similar to what I had proposed for a similar comment in 
the earlier test.

Ditto for the other occurence.



src/tests/master_tests.cpp (line 4066)


Newline before, since the comment applies to both code blocks.



src/tests/master_tests.cpp (line 4077)


s/parse1/parse



src/tests/master_tests.cpp (line 4078)


Nit: Newline before. Ditto for all the other occurences.



src/tests/master_tests.cpp (line 4084)


Nit: Newline before the multi line statement for readability.

Ditto for similar occurences later.



src/tests/master_tests.cpp (line 4099)


s/parse1/parse



src/tests/master_tests.cpp (line 4110)


Kill this and add a comment similar to what I had proposed for the earlier 
test.


- Anand Mazumdar


On Dec. 14, 2016, 4:49 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52639/
> ---
> 
> (Updated Dec. 14, 2016, 4:49 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Xiaojian Huang, and Kunal Thakar.
> 
> 
> Bugs: MESOS-6177
> https://issues.apache.org/jira/browse/MESOS-6177
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test for `recovered` AgentID and `AGENT_ADDED` after reregister.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp 82c0fc27e5e707adb73faeb26828a2ce3e3feb16 
>   src/tests/master_tests.cpp b3b5630899082a69119d2cccb6460766d1352963 
> 
> Diff: https://reviews.apache.org/r/52639/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 54732: Reused same json helper for `SlaveInfo`.

[Anand Mazumdar]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54732/#review160202
---


Ship it!




I would update the summary to be more descriptive when committing.

- Anand Mazumdar


On Dec. 14, 2016, 4:52 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54732/
> ---
> 
> (Updated Dec. 14, 2016, 4:52 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Xiaojian Huang, and Kunal Thakar.
> 
> 
> Bugs: MESOS-6177
> https://issues.apache.org/jira/browse/MESOS-6177
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reused same json helper for `SlaveInfo`.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp d52806dcf8e4d64ebb98e191a01408c0fcae17ac 
> 
> Diff: https://reviews.apache.org/r/54732/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 53344: Updated `/slaves.md` doc.

[Anand Mazumdar]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53344/#review160200
---


Ship it!




Ship It!

- Anand Mazumdar


On Dec. 14, 2016, 4:52 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53344/
> ---
> 
> (Updated Dec. 14, 2016, 4:52 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Xiaojian Huang, and Kunal Thakar.
> 
> 
> Bugs: MESOS-6177
> https://issues.apache.org/jira/browse/MESOS-6177
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated `/slaves.md` doc.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/master/slaves.md 90d0fb8496866b70777e5dbc86449b14ae1cc910 
> 
> Diff: https://reviews.apache.org/r/53344/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 52765: Populated `recovered_slaves` in `/state` and `/slaves` endpoints.

[Anand Mazumdar]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52765/#review160199
---


Ship it!




Ship It!

- Anand Mazumdar


On Dec. 14, 2016, 4:48 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52765/
> ---
> 
> (Updated Dec. 14, 2016, 4:48 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Xiaojian Huang, and Kunal Thakar.
> 
> 
> Bugs: MESOS-6177
> https://issues.apache.org/jira/browse/MESOS-6177
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Populated `recovered_slaves` in `/state` and `/slaves` endpoints.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp d52806dcf8e4d64ebb98e191a01408c0fcae17ac 
>   src/tests/master_tests.cpp b3b5630899082a69119d2cccb6460766d1352963 
> 
> Diff: https://reviews.apache.org/r/52765/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 53679: Implement capacity heuristic check related tests.

[Alexander Rukletsov]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53679/#review160148
---




src/tests/master_quota_tests.cpp (lines 1084 - 1087)


We can fit this into three lines with some rephrasing:
```
// Checks that a quota request is still satisfied even if tasks from other
// roles use up the cluster capacity, because we do not consider tasks 
running
// on unreserved resources when performing the capacity heuristic.
```



src/tests/master_quota_tests.cpp (line 1101)


Ups?



src/tests/master_quota_tests.cpp (line 1107)


Since there is a single agent in the test, enumerating it is misleading.

Also please s/slave/agent everywhere.



src/tests/master_quota_tests.cpp (lines 1120 - 1124)


Do you use `clang-format`? It suggests
```
MesosSchedulerDriver driver(
, frameworkInfo, master.get()->pid, DEFAULT_CREDENTIAL);
```



src/tests/master_quota_tests.cpp (line 1138)


Remove extra blank line. You may want to say that you would like to emulate 
a long running task consuming all available resources on the agent.



src/tests/master_quota_tests.cpp (lines 1156 - 1158)


Let's move this inside the explicit `{}` block below.



src/tests/master_quota_tests.cpp (lines 1160 - 1162)


How about this:
```
  // Submit a quota request asking for all "cpus" and "mem" resources from 
the only agent. Despite these resources are already allocated, the request 
should be satisfied because resources used by a non-quota'ed role are not 
accounted in the capacity heuristic check.
```



src/tests/master_quota_tests.cpp (lines 1182 - 1184)


Let's rephrase a bit:
```
// Checks that a quota request is satisfied even if some part of resources 
needed to fulfil the request is dynamically reserved for other role.
```



src/tests/master_quota_tests.cpp (lines 1194 - 1202)


1. If you enumerate entities, you should do it consistently to avoid 
confusion.
2. Let's be consistent in naming part of the same entity and go for 
"agent*" everywhere.
3. Let's avoid using "slave" wherever possible. Please replace all 
"*slave*" occurences with "*agent*" here and everywhere.
4. Feel free to file a separate patch to `s/slave/agent/` everywhere in 
this file to stay consistent.



src/tests/master_quota_tests.cpp (line 1207)


Kill this line.



src/tests/master_quota_tests.cpp (line 1222)


AFAIK, we don't call it "operation" when we hit the master endpoint.

Also, please backtick types, functions, and variables in comments. Also, 
keep capitalization consistent, e.g., `role1` -> `ROLE`.



src/tests/master_quota_tests.cpp (lines 1222 - 1247)


I was thinking about how we can make this snippet more readable and 
self-explaining. How about
```
Future checkpointResources =
FUTURE_PROTOBUF(CheckpointResourcesMessage(), _, slave1.get()->pid);

  // Dynamically reserve all "cpus" resources on `agent1` for `ROLE1`.
  auto reserveResourcesRequestBody = [=](const Resources& resources) -> 
string {
Resources unreserved = resources.flatten(
ROLE1,
createReservationInfo(DEFAULT_CREDENTIAL.principal())).get();

const google::protobuf::RepeatedPtrField& repeated = 
unreserved;

string reservationBody = strings::format(
"slaveId=%s=%s",
slaveId->value(),
JSON::protobuf(repeated)).get();

return reservationBody;
  };

  Future reservationResponse = process::http::post(
  master.get()->pid,
  "reserve",
  createBasicAuthHeaders(DEFAULT_CREDENTIAL),
  reserveResourcesRequestBody(agent1TotalResources->get("cpus")));
```



src/tests/master_quota_tests.cpp (lines 1223 - 1225)


Why not using `Resources::get()`?



src/tests/master_quota_tests.cpp (line 1252)


Backtick all names in comments, here and everywhere.



src/tests/master_quota_tests.cpp (lines 1256 - 1266)


Let's move this into the explicit scope below and consolidate 

Re: Review Request 53691: Implemented some quota functionality tests.

[Alexander Rukletsov]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53691/#review160153
---




src/tests/master_quota_tests.cpp (lines 1721 - 1723)


We should probably rephrase this; see my comment below.



src/tests/master_quota_tests.cpp (lines 1837 - 1839)


Yeah, current behaviour indeed prevents satisfied quota'ed frameworks from 
receiving non-revocable resources beyond their quota. It probably makes sense 
to mention this assumption in the test comment.

You don't have to trigger an allocation though, because adding an agent 
triggers one. I think it's also fine to leave a note here that we expect no 
allocations here. We don't really have to check because if an offer arrives 
here the next expectation will fail. 

What I originially had in mind was some kind of test that quota is actually 
helpful when agents start dying in the cluster. We can simulate this situation 
as well, however without quota enforcement (read: preemption and revocation) 
this test does not seem to have a lot of sense. I suggest to punt on my 
original idea for now and just go with your proposal.



src/tests/master_quota_tests.cpp (lines 1843 - 1846)


Please keep formatting consistent throughout the file.



src/tests/master_quota_tests.cpp (lines 1862 - 1864)


The expectation should be set before the event is triggered, otherwise 
there is a race: the offer will be processed and ignored and hence 
`AWAIT_READY(offers2);` will never be triggered.


- Alexander Rukletsov


On Dec. 27, 2016, 11:24 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53691/
> ---
> 
> (Updated Dec. 27, 2016, 11:24 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Xiaojian Huang.
> 
> 
> Bugs: MESOS-3982
> https://issues.apache.org/jira/browse/MESOS-3982
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This implements some tests in previous todos for
> quota functionality.
> 
> The one for:
> 
> `Role quota is below its allocation (InverseOffer generation).`
> 
> does not seem be implementable right now, since
> hierarchical allocator does not send InverseOffer for
> quota yet.
> 
> 
> Diffs
> -
> 
>   src/tests/master_quota_tests.cpp 48be7406181646c8cc1d169b82a4a4ca71cdf03b 
> 
> Diff: https://reviews.apache.org/r/53691/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 52284: Implement more quota validation tests.

[Alexander Rukletsov]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52284/#review160145
---


Fix it, then Ship it!





src/tests/master_quota_tests.cpp (lines 420 - 422)


Let's move this up right after `SetForNonExistentRole`.



src/tests/master_quota_tests.cpp (lines 447 - 448)


Let's rename the test not here but while updating it in 
https://reviews.apache.org/r/52103/


- Alexander Rukletsov


On Nov. 11, 2016, 8:55 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52284/
> ---
> 
> (Updated Nov. 11, 2016, 8:55 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Xiaojian Huang.
> 
> 
> Bugs: MESOS-4292
> https://issues.apache.org/jira/browse/MESOS-4292
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implement more quota validation tests.
> 
> 
> Diffs
> -
> 
>   src/tests/master_quota_tests.cpp 48be7406181646c8cc1d169b82a4a4ca71cdf03b 
> 
> Diff: https://reviews.apache.org/r/52284/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 53691: Implemented some quota functionality tests.

[Alexander Rukletsov]

> On Nov. 12, 2016, 8:01 a.m., Zhitao Li wrote:
> > src/tests/master_quota_tests.cpp, line 1723
> > 
> >
> > @alexr, can you please advise what is the best way for the "alert and 
> > under quota situation" check in your mind here?
> > 
> > Given that there is only one agent, there is little we could do in such 
> > a case.

I think we should punt on this for now. Originally, we were thinking about 
providing some sort of an explicit alert when allocation goes below its quota, 
but decided not to due to difficulties in determining whether to alert or not 
during failover. I'd suggest we remove this test and the corresponding TODO 
entry altogether for now.


- Alexander


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53691/#review155772
---


On Dec. 27, 2016, 11:24 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53691/
> ---
> 
> (Updated Dec. 27, 2016, 11:24 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Xiaojian Huang.
> 
> 
> Bugs: MESOS-3982
> https://issues.apache.org/jira/browse/MESOS-3982
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This implements some tests in previous todos for
> quota functionality.
> 
> The one for:
> 
> `Role quota is below its allocation (InverseOffer generation).`
> 
> does not seem be implementable right now, since
> hierarchical allocator does not send InverseOffer for
> quota yet.
> 
> 
> Diffs
> -
> 
>   src/tests/master_quota_tests.cpp 48be7406181646c8cc1d169b82a4a4ca71cdf03b 
> 
> Diff: https://reviews.apache.org/r/53691/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 55037: Added default PATH value in `launch.cpp` for Windows case.

[Till Toenshoff]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55037/#review160161
---


Fix it, then Ship it!




While the below might never be implemented for Windows I think we should still 
unify them and switch all PATH defaults to the new function.

https://github.com/apache/mesos/blob/master/src/slave/containerizer/docker.cpp#L1408
https://github.com/apache/mesos/blob/master/src/slave/containerizer/mesos/isolators/network/cni/cni.cpp#L1118
https://github.com/apache/mesos/blob/master/src/slave/containerizer/mesos/isolators/network/cni/cni.cpp#L1502


src/slave/containerizer/mesos/launch.cpp (line 141)


s/off/of/ ?



src/slave/containerizer/mesos/launch.cpp (line 148)


By not adding `syswow64` we are excluding 32bit runnables, is this 
intentional and documented? Is this still a thing on windows?


- Till Toenshoff


On Dec. 26, 2016, 9:53 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55037/
> ---
> 
> (Updated Dec. 26, 2016, 9:53 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, John Kordich, 
> and Joseph Wu.
> 
> 
> Bugs: MESOS-6839
> https://issues.apache.org/jira/browse/MESOS-6839
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, when the containerizer launches a process with the POSIX
> launcher, it will check if the `$PATH` environment variable (or `%PATH%`
> in the case of Windows is set in the `LaunchInfo`. If it is not, we
> supply a default path. Unfortunately, this default path is specific to
> POSIX. In many of our tests, this causes many of our tests to be unable
> to find Windows-standard executables like `ping`, and subsequently fail.
> 
> This commit will introduce a function, `default_path` that returns a
> sensible default path for both POSIX and Windows. Since the Windows
> implementation of this depends on the configuration of the host running
> the containerizer (rather than, say, the one creating the `TaskInfo`),
> we choose to implement this in `launch.cpp` instead of Stout, where a
> user could mistakenly call it and expect the same output on all hosts.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 
> e482ab8bdfc358f695b87cda72ca59fb64cd8c4d 
> 
> Diff: https://reviews.apache.org/r/55037/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 54993: Modify LevelDB patch to add ARM64/AArch64 support.

[Till Toenshoff]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54993/#review160160
---


Ship it!




This sums up the following changes on `atomic_pointer.h` done within the 
upstream project;
https://github.com/google/leveldb/commit/803d69203a62faf50f1b77897310a3a1fcae712b
https://github.com/google/leveldb/commit/c4c38f9c1f3bb405fe22a79c5611438f91208d09
https://github.com/google/leveldb/commit/ceff6f12152785a54885a47db349a6d8dfd0ce2c

Note that some of those changes are unrelated to ARM64 and hence not strictly 
within the scope
of this ticket -- so we do not need those changes. However, having things in 
sync with upstream
is definitely valuable and in these specific cases appears to be free of 
additional risks.

- Till Toenshoff


On Dec. 22, 2016, 9:10 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54993/
> ---
> 
> (Updated Dec. 22, 2016, 9:10 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6834
> https://issues.apache.org/jira/browse/MESOS-6834
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mesos will not compile on ARM64/AArch64 without a patch to the version of 
> LevelDB that is used within Mesos. While the fix is already in newer versions 
> of LevelDB it's not something that Mesos pulls down.
> 
> The main issue is that the AtomicPointer header needs to be aware of other 
> architectures to provide its functionality to those architectures.
> 
> 
> Diffs
> -
> 
>   3rdparty/leveldb-1.4.patch b899f0141 
> 
> Diff: https://reviews.apache.org/r/54993/diff/
> 
> 
> Testing
> ---
> 
> Compiled Mesos on ARM64 with no failures. Mesos also properly starts and 
> successfully runs/launches tasks with the exception of a crash when using the 
> linux_launcher and Mesos containers. That fix is addressed in 
> https://reviews.apache.org/r/54996/
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 54996: Fix SIGBUS crash on ARM64/AArch64.

[Till Toenshoff]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54996/#review160159
---




3rdparty/stout/include/stout/os/linux.hpp (line 81)


As you have no further use for the returned value, I guess we could simply 
spare the temporary and make it a bit more concise and also C++'esque by 
prventing the C-style cast;

```
  if (posix_memalign(static_cast(>address), 16, stack->size) 
!=
  0) {
return -1;
  }
```


- Till Toenshoff


On Dec. 22, 2016, 9:24 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54996/
> ---
> 
> (Updated Dec. 22, 2016, 9:24 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6835
> https://issues.apache.org/jira/browse/MESOS-6835
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently in the Linux launcher when the stack is allocated and prepared for 
> a call to clone() it is not properly aligned. This is not an issue for x86 or 
> x64 but for ARM64/AArch64 it is because of the requirement of having the 
> stack aligned to a 16 byte boundary. While x86 and x64 also expect the stack 
> to have a 16 byte aligned stack, it is not enforced. An explanation of the 
> stack and requirements for ARM64 can be found here 
> http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf
>  (specifically section 5.2.2.1 that says SP mod 16 = 0. The stack must be 
> quad-word aligned.)
> 
> Additionally, the way that the stack is currently allocated and passed to 
> clone() accidentally chops off one entry, making a stack overflow using those 
> missing 8 bytes a possibility. Fixing this while aligning the memory will fix 
> both the issue of the stack overflow issue as well as the SIGBUS crash.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/linux.hpp 530f1a55b 
> 
> Diff: https://reviews.apache.org/r/54996/diff/
> 
> 
> Testing
> ---
> 
> Built Mesos from source and am currently running it in a test cluster. 
> Launched both Docker and Mesos tasks via Marathon without any resulting crash 
> (initial crash only happened with Mesos containerizer + linux_launcher, not 
> with the posix_launcher).
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 55006: CMake: renamed test binaries to match autotools.

[Till Toenshoff]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55006/#review160157
---


Ship it!




Ship It!

- Till Toenshoff


On Dec. 23, 2016, 2:22 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55006/
> ---
> 
> (Updated Dec. 23, 2016, 2:22 a.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently CMake generates test binaries named `stout_tests`,
> `process_tests` (underscores), and `mesos-tests` (dash).
> 
> We should simply follow the naming from autotools and call them
> `stout-tests`, `libprocess-tests`, and `mesos-tests`.
> 
> 
> Diffs
> -
> 
>   src/tests/CMakeLists.txt b034f1b88337b003d01450eadd262b9bc763545c 
>   support/windows-build.bat 4d1d8cd6cdcf6acac259b570387923fe385f4790 
> 
> Diff: https://reviews.apache.org/r/55006/diff/
> 
> 
> Testing
> ---
> 
> `cmake --build . -- -j 6`
> ```
> $ find . -name "*[-_]tests"
> ./3rdparty/libprocess/src/tests/libprocess-tests
> ./3rdparty/stout/tests/stout-tests
> ./src/mesos-tests
> ```
> 
> Built on Windows via `support/windows-build.bat`.
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 55005: CMake: renamed `process_tests` to `libprocess-tests` to match autotools.

[Till Toenshoff]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55005/#review160156
---


Fix it, then Ship it!





3rdparty/libprocess/src/tests/CMakeLists.txt (line 67)


s/process-tests/libprocess-tests/ ?


- Till Toenshoff


On Dec. 23, 2016, 2:09 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55005/
> ---
> 
> (Updated Dec. 23, 2016, 2:09 a.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See https://reviews.apache.org/r/55006/.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
> 9817426c3c924c7cd988d5bf4fd41b6a2fd4962a 
>   3rdparty/libprocess/src/tests/CMakeLists.txt 
> 4b80c397381ca2c869cd6eb7507bb9df94ce3623 
> 
> Diff: https://reviews.apache.org/r/55005/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 55004: CMake: renamed `stout_tests` to `stout-tests` to match autotools.

[Till Toenshoff]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55004/#review160155
---


Ship it!




Ship It!

- Till Toenshoff


On Dec. 23, 2016, 2:09 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55004/
> ---
> 
> (Updated Dec. 23, 2016, 2:09 a.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See https://reviews.apache.org/r/55006/.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/cmake/StoutTestsConfigure.cmake 
> d62fe727f075ed9ef5e626ad4d108dfafd8c2fec 
>   3rdparty/stout/tests/CMakeLists.txt 
> a09693d4b9a70faf1e4e337ec71ead2f0bbab0a2 
> 
> Diff: https://reviews.apache.org/r/55004/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 53691: Implemented some quota functionality tests.

[Zhitao Li]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53691/
---

(Updated Dec. 27, 2016, 11:24 a.m.)


Review request for mesos, Alexander Rukletsov and Xiaojian Huang.


Bugs: MESOS-3982
https://issues.apache.org/jira/browse/MESOS-3982


Repository: mesos


Description
---

This implements some tests in previous todos for
quota functionality.

The one for:

`Role quota is below its allocation (InverseOffer generation).`

does not seem be implementable right now, since
hierarchical allocator does not send InverseOffer for
quota yet.


Diffs
-

  src/tests/master_quota_tests.cpp 48be7406181646c8cc1d169b82a4a4ca71cdf03b 

Diff: https://reviews.apache.org/r/53691/diff/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 53679: Implement capacity heuristic check related tests.

[Zhitao Li]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53679/
---

(Updated Dec. 27, 2016, 11:23 a.m.)


Review request for mesos, Alexander Rukletsov, Xiaojian Huang, and Kunal Thakar.


Bugs: MESOS-6840
https://issues.apache.org/jira/browse/MESOS-6840


Repository: mesos


Description
---

Implement capacity heuristic check related tests.


Diffs
-

  src/tests/master_quota_tests.cpp 48be7406181646c8cc1d169b82a4a4ca71cdf03b 

Diff: https://reviews.apache.org/r/53679/diff/


Testing
---


Thanks,

Zhitao Li