Re: Review Request 69426: Replaced CLI test helper function 'running_tasks' by 'wait_for_task'.

2018-11-26 Thread Armand Grillet 

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

(Updated Nov. 26, 2018, 2:45 p.m.)


Review request for mesos and Kevin Klues.


Changes
---

Fixed issues.


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


Repository: mesos


Description
---

Replaces 'running_tasks(master)', a function that was not generic nor
explicit, by 'wait_for_task(master, name, state, delay)'. This helper
function waits a 'delay' for a task with a given 'name' to be in a
certain 'state'.

All uses of 'running_tasks' have been replaced by the new function.


Diffs (updated)
-

  src/python/cli_new/lib/cli/tests/base.py 
58c96d7ba45a57cb824d1f7f146e6dc5e41572cd 
  src/python/cli_new/lib/cli/tests/task.py 
eb01bf5cae095a067572f534a13f4ec542101ca5 


Diff: https://reviews.apache.org/r/69426/diff/3/

Changes: https://reviews.apache.org/r/69426/diff/2-3/


Testing
---

```
(mesos-cli) ?  cli_new (MESOS-9399) ? mesos-cli-tests
Running the Mesos CLI unit tests

TestAgentPlugin
test_list (cli.tests.agent.TestAgentPlugin) ... ok

TestInfrastructure
test_capture_output (cli.tests.tests.TestInfrastructure) ... ok
test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok

TestTaskPlugin
test_exec (cli.tests.task.TestTaskPlugin) ... ok
test_exec_exit_status (cli.tests.task.TestTaskPlugin) ... ok
test_exec_interactive (cli.tests.task.TestTaskPlugin) ... ok
test_list (cli.tests.task.TestTaskPlugin) ... ok

--
Ran 7 tests in 19.319s

OK
```


Thanks,

Armand Grillet

Re: Review Request 69426: Replaced CLI test helper function 'running_tasks' by 'wait_for_task'.

2018-11-26 Thread Kevin Klues 

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




src/python/cli_new/lib/cli/tests/base.py
Lines 516 (patched)


Let's return the task here. I'm not 100% sure when / how we will use it, 
but it's better than just retunring `None` in my opinion.



src/python/cli_new/lib/cli/tests/task.py
Lines 70 (patched)


Add the name of the task in the error message.



src/python/cli_new/lib/cli/tests/task.py
Lines 77 (patched)


This could fail for two reasons (which is fine), but we should change the 
error message to reflect exactly what's gone wrong (i.e. that we can't get the 
tasks from that endpoint -- not just that we couldn't open it).



src/python/cli_new/lib/cli/tests/task.py
Lines 78 (patched)


Let's put a line break here.



src/python/cli_new/lib/cli/tests/task.py
Lines 96-99 (original), 108-122 (patched)


Same comments as above.



src/python/cli_new/lib/cli/tests/task.py
Lines 129-132 (original), 152-166 (patched)


Same comments as above.



src/python/cli_new/lib/cli/tests/task.py
Lines 164-168 (original), 198-210 (patched)


Same comments as above.


- Kevin Klues


On Nov. 25, 2018, 11:08 vorm., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69426/
> ---
> 
> (Updated Nov. 25, 2018, 11:08 vorm.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-9399
> https://issues.apache.org/jira/browse/MESOS-9399
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replaces 'running_tasks(master)', a function that was not generic nor
> explicit, by 'wait_for_task(master, name, state, delay)'. This helper
> function waits a 'delay' for a task with a given 'name' to be in a
> certain 'state'.
> 
> All uses of 'running_tasks' have been replaced by the new function.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/lib/cli/tests/base.py 
> c28e2a65b5c1b881c2f4eb09baf9806338f26e40 
>   src/python/cli_new/lib/cli/tests/task.py 
> 1b48c0a4729043c874330552074b05368e38b715 
> 
> 
> Diff: https://reviews.apache.org/r/69426/diff/2/
> 
> 
> Testing
> ---
> 
> ```
> (mesos-cli) ?  cli_new (MESOS-9399) ? mesos-cli-tests
> Running the Mesos CLI unit tests
> 
> TestAgentPlugin
> test_list (cli.tests.agent.TestAgentPlugin) ... ok
> 
> TestInfrastructure
> test_capture_output (cli.tests.tests.TestInfrastructure) ... ok
> test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok
> 
> TestTaskPlugin
> test_exec (cli.tests.task.TestTaskPlugin) ... ok
> test_exec_exit_status (cli.tests.task.TestTaskPlugin) ... ok
> test_exec_interactive (cli.tests.task.TestTaskPlugin) ... ok
> test_list (cli.tests.task.TestTaskPlugin) ... ok
> 
> --
> Ran 7 tests in 19.319s
> 
> OK
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 69426: Replaced CLI test helper function 'running_tasks' by 'wait_for_task'.

2018-11-25 Thread Armand Grillet 

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

(Updated Nov. 25, 2018, 12:08 p.m.)


Review request for mesos and Kevin Klues.


Changes
---

Fixed issues.


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


Repository: mesos


Description
---

Replaces 'running_tasks(master)', a function that was not generic nor
explicit, by 'wait_for_task(master, name, state, delay)'. This helper
function waits a 'delay' for a task with a given 'name' to be in a
certain 'state'.

All uses of 'running_tasks' have been replaced by the new function.


Diffs (updated)
-

  src/python/cli_new/lib/cli/tests/base.py 
c28e2a65b5c1b881c2f4eb09baf9806338f26e40 
  src/python/cli_new/lib/cli/tests/task.py 
1b48c0a4729043c874330552074b05368e38b715 


Diff: https://reviews.apache.org/r/69426/diff/2/

Changes: https://reviews.apache.org/r/69426/diff/1-2/


Testing
---

```
(mesos-cli) ?  cli_new (MESOS-9399) ? mesos-cli-tests
Running the Mesos CLI unit tests

TestAgentPlugin
test_list (cli.tests.agent.TestAgentPlugin) ... ok

TestInfrastructure
test_capture_output (cli.tests.tests.TestInfrastructure) ... ok
test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok

TestTaskPlugin
test_exec (cli.tests.task.TestTaskPlugin) ... ok
test_exec_exit_status (cli.tests.task.TestTaskPlugin) ... ok
test_exec_interactive (cli.tests.task.TestTaskPlugin) ... ok
test_list (cli.tests.task.TestTaskPlugin) ... ok

--
Ran 7 tests in 19.319s

OK
```


Thanks,

Armand Grillet

Re: Review Request 69426: Replaced CLI test helper function 'running_tasks' by 'wait_for_task'.

2018-11-22 Thread Kevin Klues 

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




src/python/cli_new/lib/cli/tests/task.py
Lines 66-68 (original), 68-70 (patched)


Instead of throwing an exception here, do an assert that the tasks array 
has 1 element in it.

Do the same for all instances below.


- Kevin Klues


On Nov. 22, 2018, 11:35 vorm., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69426/
> ---
> 
> (Updated Nov. 22, 2018, 11:35 vorm.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-9399
> https://issues.apache.org/jira/browse/MESOS-9399
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replaces 'running_tasks(master)', a function that was not generic nor
> explicit, by 'wait_for_task(master, name, state, delay)'. This helper
> function waits a 'delay' for a task with a given 'name' to be in a
> certain 'state'.
> 
> All uses of 'running_tasks' have been replaced by the new function.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/lib/cli/tests/base.py 
> c28e2a65b5c1b881c2f4eb09baf9806338f26e40 
>   src/python/cli_new/lib/cli/tests/task.py 
> f033b4a7663279c8a7c49ed86b3acdda45ceb107 
> 
> 
> Diff: https://reviews.apache.org/r/69426/diff/1/
> 
> 
> Testing
> ---
> 
> ```
> (mesos-cli) ?  cli_new (MESOS-9399) ? mesos-cli-tests
> Running the Mesos CLI unit tests
> 
> TestAgentPlugin
> test_list (cli.tests.agent.TestAgentPlugin) ... ok
> 
> TestInfrastructure
> test_capture_output (cli.tests.tests.TestInfrastructure) ... ok
> test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok
> 
> TestTaskPlugin
> test_exec (cli.tests.task.TestTaskPlugin) ... ok
> test_exec_exit_status (cli.tests.task.TestTaskPlugin) ... ok
> test_exec_interactive (cli.tests.task.TestTaskPlugin) ... ok
> test_list (cli.tests.task.TestTaskPlugin) ... ok
> 
> --
> Ran 7 tests in 19.319s
> 
> OK
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 69426: Replaced CLI test helper function 'running_tasks' by 'wait_for_task'.

2018-11-22 Thread Kevin Klues 

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




src/python/cli_new/lib/cli/tests/base.py
Line 506 (original), 507 (patched)


Add a default dealy of 1 here, so that tests don't always have to write it 
in.



src/python/cli_new/lib/cli/tests/base.py
Line 521 (original), 522 (patched)


This should be a CLIException and should have a string saying:
```
"Timed out waiting for task"
```



src/python/cli_new/lib/cli/tests/task.py
Lines 65-68 (original), 66-70 (patched)


The new `wait_or_task` call should be wrapped in a `try` and the 
`CLIException` it raises should be composed according to the normal composition 
rules we use for exceptions.

We also don't need the delay set explicitly to `1` anymore if you make the 
change mentioned in the previous file.

Apply this to all instance of this pattern below as well.


- Kevin Klues


On Nov. 22, 2018, 11:35 vorm., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69426/
> ---
> 
> (Updated Nov. 22, 2018, 11:35 vorm.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-9399
> https://issues.apache.org/jira/browse/MESOS-9399
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replaces 'running_tasks(master)', a function that was not generic nor
> explicit, by 'wait_for_task(master, name, state, delay)'. This helper
> function waits a 'delay' for a task with a given 'name' to be in a
> certain 'state'.
> 
> All uses of 'running_tasks' have been replaced by the new function.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/lib/cli/tests/base.py 
> c28e2a65b5c1b881c2f4eb09baf9806338f26e40 
>   src/python/cli_new/lib/cli/tests/task.py 
> f033b4a7663279c8a7c49ed86b3acdda45ceb107 
> 
> 
> Diff: https://reviews.apache.org/r/69426/diff/1/
> 
> 
> Testing
> ---
> 
> ```
> (mesos-cli) ?  cli_new (MESOS-9399) ? mesos-cli-tests
> Running the Mesos CLI unit tests
> 
> TestAgentPlugin
> test_list (cli.tests.agent.TestAgentPlugin) ... ok
> 
> TestInfrastructure
> test_capture_output (cli.tests.tests.TestInfrastructure) ... ok
> test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok
> 
> TestTaskPlugin
> test_exec (cli.tests.task.TestTaskPlugin) ... ok
> test_exec_exit_status (cli.tests.task.TestTaskPlugin) ... ok
> test_exec_interactive (cli.tests.task.TestTaskPlugin) ... ok
> test_list (cli.tests.task.TestTaskPlugin) ... ok
> 
> --
> Ran 7 tests in 19.319s
> 
> OK
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 69426: Replaced CLI test helper function 'running_tasks' by 'wait_for_task'.

2018-11-22 Thread Mesos Reviewbot Windows 

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



FAIL: Failed to apply the dependent review: 69394.

Failed command: `python.exe .\support\apply-reviews.py -n -r 69394`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2635/mesos-review-69426

Relevant logs:

- 
[apply-review-69394.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2635/mesos-review-69426/logs/apply-review-69394.log):

```
error: patch failed: src/python/cli_new/lib/cli/plugins/task/main.py:126
error: src/python/cli_new/lib/cli/plugins/task/main.py: patch does not apply
error: patch failed: src/python/cli_new/lib/cli/tests/task.py:186
error: src/python/cli_new/lib/cli/tests/task.py: patch does not apply
```

- Mesos Reviewbot Windows


On Nov. 22, 2018, 12:35 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69426/
> ---
> 
> (Updated Nov. 22, 2018, 12:35 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-9399
> https://issues.apache.org/jira/browse/MESOS-9399
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replaces 'running_tasks(master)', a function that was not generic nor
> explicit, by 'wait_for_task(master, name, state, delay)'. This helper
> function waits a 'delay' for a task with a given 'name' to be in a
> certain 'state'.
> 
> All uses of 'running_tasks' have been replaced by the new function.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/lib/cli/tests/base.py 
> c28e2a65b5c1b881c2f4eb09baf9806338f26e40 
>   src/python/cli_new/lib/cli/tests/task.py 
> f033b4a7663279c8a7c49ed86b3acdda45ceb107 
> 
> 
> Diff: https://reviews.apache.org/r/69426/diff/1/
> 
> 
> Testing
> ---
> 
> ```
> (mesos-cli) ?  cli_new (MESOS-9399) ? mesos-cli-tests
> Running the Mesos CLI unit tests
> 
> TestAgentPlugin
> test_list (cli.tests.agent.TestAgentPlugin) ... ok
> 
> TestInfrastructure
> test_capture_output (cli.tests.tests.TestInfrastructure) ... ok
> test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok
> 
> TestTaskPlugin
> test_exec (cli.tests.task.TestTaskPlugin) ... ok
> test_exec_exit_status (cli.tests.task.TestTaskPlugin) ... ok
> test_exec_interactive (cli.tests.task.TestTaskPlugin) ... ok
> test_list (cli.tests.task.TestTaskPlugin) ... ok
> 
> --
> Ran 7 tests in 19.319s
> 
> OK
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>