Re: Review Request 49448: Added testcases for LIST_FILES call.

2016-07-07 Thread Abhishek Dasgupta 

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

(Updated July 7, 2016, 5:35 p.m.)


Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and Vinod 
Kone.


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


Repository: mesos


Description
---

Added testcases for LIST_FILES call.


Diffs (updated)
-

  src/tests/api_tests.cpp 7cf716dd7a5def8d648b1d8dbc2b9d5c158b00e9 

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


Testing
---

On Ubuntu 16.04:
sudo 
GTEST_FILTER="*AgentAPITest.ListFiles*:FilesTest.BrowseTest:*MasterAPITest.ListFiles*"
 make -j4 check


Thanks,

Abhishek Dasgupta

Re: Review Request 49448: Added testcases for LIST_FILES call.

2016-07-07 Thread Abhishek Dasgupta 

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

(Updated July 7, 2016, 4:07 p.m.)


Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and Vinod 
Kone.


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


Repository: mesos


Description
---

Added testcases for LIST_FILES call.


Diffs (updated)
-

  src/tests/api_tests.cpp 7cf716dd7a5def8d648b1d8dbc2b9d5c158b00e9 

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


Testing
---

On Ubuntu 16.04:
sudo 
GTEST_FILTER="*AgentAPITest.ListFiles*:FilesTest.BrowseTest:*MasterAPITest.ListFiles*"
 make -j4 check


Thanks,

Abhishek Dasgupta

Re: Review Request 49448: Added testcases for LIST_FILES call.

2016-07-07 Thread Mesos ReviewBot 

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



Patch looks great!

Reviews applied: [49301, 49443, 49444, 49445, 49696, 49529, 49446, 49447, 
49550, 49551, 49448]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On July 7, 2016, 1:42 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49448/
> ---
> 
> (Updated July 7, 2016, 1:42 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5514
> https://issues.apache.org/jira/browse/MESOS-5514
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added testcases for LIST_FILES call.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp 7cf716dd7a5def8d648b1d8dbc2b9d5c158b00e9 
> 
> Diff: https://reviews.apache.org/r/49448/diff/
> 
> 
> Testing
> ---
> 
> On Ubuntu 16.04:
> sudo 
> GTEST_FILTER="*AgentAPITest.ListFiles*:FilesTest.BrowseTest:*MasterAPITest.ListFiles*"
>  make -j4 check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 49448: Added testcases for LIST_FILES call.

2016-07-07 Thread Anand Mazumdar 

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


Fix it, then Ship it!




Can you run the newly introduced tests with `gtest_repeat=100` to ensure they 
don't show any flakiness.


src/tests/api_tests.cpp (line 2276)


hmm, this is flaky!

The future `__recover` would be completed as soon as `&Slave::__recover()` 
is invoked but not before it has _completed_. Hence, if the `post()` happens 
before the recovery is complete. This test would fail with a `503`.

You need this after the `AWAIT_*`
```cpp
// Wait for recovery to be complete.
  Clock::pause();
  Clock::settle();
```

Ditto for the other agent test.


- Anand Mazumdar


On July 7, 2016, 1:42 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49448/
> ---
> 
> (Updated July 7, 2016, 1:42 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5514
> https://issues.apache.org/jira/browse/MESOS-5514
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added testcases for LIST_FILES call.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp 7cf716dd7a5def8d648b1d8dbc2b9d5c158b00e9 
> 
> Diff: https://reviews.apache.org/r/49448/diff/
> 
> 
> Testing
> ---
> 
> On Ubuntu 16.04:
> sudo 
> GTEST_FILTER="*AgentAPITest.ListFiles*:FilesTest.BrowseTest:*MasterAPITest.ListFiles*"
>  make -j4 check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 49448: Added testcases for LIST_FILES call.

2016-07-07 Thread Abhishek Dasgupta 

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

(Updated July 7, 2016, 1:42 p.m.)


Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and Vinod 
Kone.


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


Repository: mesos


Description
---

Added testcases for LIST_FILES call.


Diffs (updated)
-

  src/tests/api_tests.cpp 7cf716dd7a5def8d648b1d8dbc2b9d5c158b00e9 

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


Testing
---

On Ubuntu 16.04:
sudo 
GTEST_FILTER="*AgentAPITest.ListFiles*:FilesTest.BrowseTest:*MasterAPITest.ListFiles*"
 make -j4 check


Thanks,

Abhishek Dasgupta

Re: Review Request 49448: Added testcases for LIST_FILES call.

2016-07-06 Thread Mesos ReviewBot 

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



Bad patch!

Reviews applied: [49448, 49551, 49550, 49447, 49446, 49529, 49696, 49445, 
49444, 49443, 49301]

Failed command: ./support/apply-review.sh -n -r 49445

Error:
2016-07-06 23:49:05 URL:https://reviews.apache.org/r/49445/diff/raw/ 
[9674/9674] -> "49445.patch" [1]
error: patch failed: src/files/files.hpp:17
error: src/files/files.hpp: patch does not apply
error: patch failed: src/files/files.cpp:53
error: src/files/files.cpp: patch does not apply

Full log: https://builds.apache.org/job/mesos-reviewbot/14189/console

- Mesos ReviewBot


On July 6, 2016, 4:42 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49448/
> ---
> 
> (Updated July 6, 2016, 4:42 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5514
> https://issues.apache.org/jira/browse/MESOS-5514
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added testcases for LIST_FILES call.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp 7cf716dd7a5def8d648b1d8dbc2b9d5c158b00e9 
> 
> Diff: https://reviews.apache.org/r/49448/diff/
> 
> 
> Testing
> ---
> 
> On Ubuntu 16.04:
> sudo 
> GTEST_FILTER="*AgentAPITest.ListFiles*:FilesTest.BrowseTest:*MasterAPITest.ListFiles*"
>  make -j4 check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 49448: Added testcases for LIST_FILES call.

2016-07-06 Thread Anand Mazumdar 

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


Fix it, then Ship it!





src/tests/api_tests.cpp (line 785)


s/ListFilesSuccessful/ListFiles



src/tests/api_tests.cpp (lines 821 - 822)


How about:
// This test verifies that the client will receive a `NotFound` response 
when it tries to make a `LIST_FILES` call with an invalid path.

Ditto for the agent test.



src/tests/api_tests.cpp (line 823)


ListFilesUnsuccessful/ListFilesInvalidPath



src/tests/api_tests.cpp (lines 827 - 836)


Do you need these for this test?


- Anand Mazumdar


On July 6, 2016, 4:42 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49448/
> ---
> 
> (Updated July 6, 2016, 4:42 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5514
> https://issues.apache.org/jira/browse/MESOS-5514
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added testcases for LIST_FILES call.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp 7cf716dd7a5def8d648b1d8dbc2b9d5c158b00e9 
> 
> Diff: https://reviews.apache.org/r/49448/diff/
> 
> 
> Testing
> ---
> 
> On Ubuntu 16.04:
> sudo 
> GTEST_FILTER="*AgentAPITest.ListFiles*:FilesTest.BrowseTest:*MasterAPITest.ListFiles*"
>  make -j4 check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 49448: Added testcases for LIST_FILES call.

2016-07-06 Thread Abhishek Dasgupta 

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

(Updated July 6, 2016, 4:42 p.m.)


Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and Vinod 
Kone.


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


Repository: mesos


Description
---

Added testcases for LIST_FILES call.


Diffs (updated)
-

  src/tests/api_tests.cpp 7cf716dd7a5def8d648b1d8dbc2b9d5c158b00e9 

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


Testing
---

On Ubuntu 16.04:
sudo 
GTEST_FILTER="*AgentAPITest.ListFiles*:FilesTest.BrowseTest:*MasterAPITest.ListFiles*"
 make -j4 check


Thanks,

Abhishek Dasgupta

Re: Review Request 49448: Added testcases for LIST_FILES call.

2016-07-05 Thread Anand Mazumdar 

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




src/tests/api_tests.cpp (line 537)


Kill this after separating this test into 2.



src/tests/api_tests.cpp (lines 546 - 551)


Indent incorrect.



src/tests/api_tests.cpp (lines 554 - 568)


It's a bit odd that the test header comment says that it tries to verify if 
it can retrieve the file listing. :-)

Let's move this into a separate test rather than merging it here.

Ditto for the agent test.


- Anand Mazumdar


On July 2, 2016, 7:50 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49448/
> ---
> 
> (Updated July 2, 2016, 7:50 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5514
> https://issues.apache.org/jira/browse/MESOS-5514
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added testcases for LIST_FILES call.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp b3127b716480223a6f23b7908bf6bc1808120f80 
> 
> Diff: https://reviews.apache.org/r/49448/diff/
> 
> 
> Testing
> ---
> 
> On Ubuntu 16.04:
> sudo 
> GTEST_FILTER="*AgentAPITest.ListFiles*:FilesTest.BrowseTest:*MasterAPITest.ListFiles*"
>  make -j4 check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 49448: Added testcases for LIST_FILES call.

2016-07-02 Thread haosdent huang 

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


Ship it!




Ship It!

- haosdent huang


On July 2, 2016, 7:50 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49448/
> ---
> 
> (Updated July 2, 2016, 7:50 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5514
> https://issues.apache.org/jira/browse/MESOS-5514
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added testcases for LIST_FILES call.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp b3127b716480223a6f23b7908bf6bc1808120f80 
> 
> Diff: https://reviews.apache.org/r/49448/diff/
> 
> 
> Testing
> ---
> 
> On Ubuntu 16.04:
> sudo 
> GTEST_FILTER="*AgentAPITest.ListFiles*:FilesTest.BrowseTest:*MasterAPITest.ListFiles*"
>  make -j4 check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 49448: Added testcases for LIST_FILES call.

2016-07-02 Thread Mesos ReviewBot 

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



Patch looks great!

Reviews applied: [49301, 49443, 49444, 49445, 49529, 49446, 49447, 49550, 
49551, 49448]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On July 2, 2016, 7:50 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49448/
> ---
> 
> (Updated July 2, 2016, 7:50 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5514
> https://issues.apache.org/jira/browse/MESOS-5514
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added testcases for LIST_FILES call.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp b3127b716480223a6f23b7908bf6bc1808120f80 
> 
> Diff: https://reviews.apache.org/r/49448/diff/
> 
> 
> Testing
> ---
> 
> On Ubuntu 16.04:
> sudo 
> GTEST_FILTER="*AgentAPITest.ListFiles*:FilesTest.BrowseTest:*MasterAPITest.ListFiles*"
>  make -j4 check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 49448: Added testcases for LIST_FILES call.

2016-07-02 Thread Abhishek Dasgupta 

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

(Updated July 2, 2016, 7:50 a.m.)


Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and Vinod 
Kone.


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


Repository: mesos


Description
---

Added testcases for LIST_FILES call.


Diffs (updated)
-

  src/tests/api_tests.cpp b3127b716480223a6f23b7908bf6bc1808120f80 

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


Testing
---

On Ubuntu 16.04:
sudo 
GTEST_FILTER="*AgentAPITest.ListFiles*:FilesTest.BrowseTest:*MasterAPITest.ListFiles*"
 make -j4 check


Thanks,

Abhishek Dasgupta

Re: Review Request 49448: Added testcases for LIST_FILES call.

2016-07-02 Thread Abhishek Dasgupta 

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

(Updated July 2, 2016, 7:45 a.m.)


Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and Vinod 
Kone.


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


Repository: mesos


Description
---

Added testcases for LIST_FILES call.


Diffs (updated)
-

  src/tests/api_tests.cpp b3127b716480223a6f23b7908bf6bc1808120f80 

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


Testing
---

On Ubuntu 16.04:
sudo 
GTEST_FILTER="*AgentAPITest.ListFiles*:FilesTest.BrowseTest:*MasterAPITest.ListFiles*"
 make -j4 check


Thanks,

Abhishek Dasgupta

Re: Review Request 49448: Added testcases for LIST_FILES call.

2016-06-30 Thread Anand Mazumdar 

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



LGTM!

- Can we add a test for the unhappy case too i.e. 
`BadRequest`/`InternalServerError` on either of master/agent?
- Similar comments as for the Master test also apply to the agent test. I did 
not duplicate the comments.


include/mesos/v1/mesos.hpp (line 55)


These should be part of a separate review since these are not related to 
adding tests.



src/internal/evolve.hpp (line 72)


These should be part of a separate review since these are not related to 
adding tests.



src/tests/api_tests.cpp (line 514)


s/correct file information in a specified path/file listing for a directory



src/tests/api_tests.cpp (line 519)


hmm, why do you need this?



src/tests/api_tests.cpp (line 527)


Missing backticks around `FileInfo`.

s/FileInfo message for a/`FileInfo` for

s/a specified path/"1/two" file.

Ditto for the agent test.



src/tests/api_tests.cpp (line 532)


Nit: `this->` looks redundant. Just `StartMaster` should suffice?



src/tests/api_tests.cpp (line 549)


Align with `internal::evolve`


- Anand Mazumdar


On June 30, 2016, 2:07 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49448/
> ---
> 
> (Updated June 30, 2016, 2:07 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5514
> https://issues.apache.org/jira/browse/MESOS-5514
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added testcases for LIST_FILES call.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/mesos.hpp d220966bb9464595c927474532ccdcaf41690ef6 
>   src/internal/evolve.hpp 582f3476d0f61c9356cf5aff5f242f3e8103822d 
>   src/internal/evolve.cpp bb99cb29db84e5476a820b261ff24348c657bae0 
>   src/tests/api_tests.cpp b3127b716480223a6f23b7908bf6bc1808120f80 
>   src/v1/mesos.cpp 30788dcc1dd744553ddd93d23b11fcce44502b46 
> 
> Diff: https://reviews.apache.org/r/49448/diff/
> 
> 
> Testing
> ---
> 
> On Ubuntu 16.04:
> sudo 
> GTEST_FILTER="*AgentAPITest.ListFiles*:FilesTest.BrowseTest:*MasterAPITest.ListFiles*"
>  make -j4 check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 49448: Added testcases for LIST_FILES call.

2016-06-30 Thread Mesos ReviewBot 

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



Patch looks great!

Reviews applied: [49301, 49443, 49444, 49445, 49446, 49447, 49448]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On June 30, 2016, 2:07 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49448/
> ---
> 
> (Updated June 30, 2016, 2:07 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5514
> https://issues.apache.org/jira/browse/MESOS-5514
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added testcases for LIST_FILES call.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/mesos.hpp d220966bb9464595c927474532ccdcaf41690ef6 
>   src/internal/evolve.hpp 582f3476d0f61c9356cf5aff5f242f3e8103822d 
>   src/internal/evolve.cpp bb99cb29db84e5476a820b261ff24348c657bae0 
>   src/tests/api_tests.cpp b3127b716480223a6f23b7908bf6bc1808120f80 
>   src/v1/mesos.cpp 30788dcc1dd744553ddd93d23b11fcce44502b46 
> 
> Diff: https://reviews.apache.org/r/49448/diff/
> 
> 
> Testing
> ---
> 
> On Ubuntu 16.04:
> sudo 
> GTEST_FILTER="*AgentAPITest.ListFiles*:FilesTest.BrowseTest:*MasterAPITest.ListFiles*"
>  make -j4 check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>