swarming: task_runner fails to handle timeout |
||
Issue descriptionkill_and_wait() never worked. While fixing is trivial, we should add a proper test to ensure this works. https://chromium.googlesource.com/infra/luci/luci-py.git/+/fbd748ec330193cfbee4baa2c28345c3588388d0/appengine/swarming/swarming_bot/bot_code/task_runner.py#353
,
Jan 15
The following revision refers to this bug: https://chromium.googlesource.com/infra/luci/luci-py.git/+/55f70ce5ad7eec605742927318868e99245aff2e commit 55f70ce5ad7eec605742927318868e99245aff2e Author: Marc-Antoine Ruel <maruel@chromium.org> Date: Tue Jan 15 20:11:16 2019 client: lower symlink message to warning When it is logged as an error on Windows, it shows up in the task's stdout and breaks many task_runner_test.py test cases on Windows. These failures make it hard to add more test cases. Bug: 921842 Change-Id: I2304ab8bf4955b9332f8bb3c0a1e737a9c039b4a Reviewed-on: https://chromium-review.googlesource.com/c/1412256 Auto-Submit: Marc-Antoine Ruel <maruel@chromium.org> Reviewed-by: Quinten Yearsley <qyearsley@chromium.org> Commit-Queue: Quinten Yearsley <qyearsley@chromium.org> [modify] https://crrev.com/55f70ce5ad7eec605742927318868e99245aff2e/client/isolateserver.py [modify] https://crrev.com/55f70ce5ad7eec605742927318868e99245aff2e/client/run_isolated.py
,
Jan 15
The following revision refers to this bug: https://chromium.googlesource.com/infra/luci/luci-py.git/+/e2326f7d43117dc43524a002e5f9d44f949e1fe2 commit e2326f7d43117dc43524a002e5f9d44f949e1fe2 Author: Marc-Antoine Ruel <maruel@chromium.org> Date: Tue Jan 15 21:29:39 2019 swarming_bot: rename fake_swarming, migrate to httpserver fake_swarming was written a long time ago. Update it to reuse httpserver, which works well. This enables reducing the amount of duplicate code. Rename it to use the same naming convention than in client/tests/. Reorder the imports in task_runner_test.py to make it clearer what comes from where. This work is split out as this separate change, as the next one is to update task_runner_test.py to use this, instead of mocked HTTP request expectations, and this one will be fairly involved. Bug: 921842 Change-Id: I85086428627287d2ddeb8587f59a541009d10d73 Reviewed-on: https://chromium-review.googlesource.com/c/1412258 Commit-Queue: Marc-Antoine Ruel <maruel@chromium.org> Commit-Queue: Quinten Yearsley <qyearsley@chromium.org> Reviewed-by: Quinten Yearsley <qyearsley@chromium.org> [modify] https://crrev.com/e2326f7d43117dc43524a002e5f9d44f949e1fe2/appengine/swarming/swarming_bot/bot_code/task_runner_test.py [delete] https://crrev.com/55f70ce5ad7eec605742927318868e99245aff2e/appengine/swarming/swarming_bot/fake_swarming.py [modify] https://crrev.com/e2326f7d43117dc43524a002e5f9d44f949e1fe2/appengine/swarming/swarming_bot/main_test.py [add] https://crrev.com/e2326f7d43117dc43524a002e5f9d44f949e1fe2/appengine/swarming/swarming_bot/swarmingserver_bot_fake.py
,
Jan 17
(6 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/infra/luci/luci-py.git/+/fac63cee56e1659cc3e8360f77603aaaac27c28b commit fac63cee56e1659cc3e8360f77603aaaac27c28b Author: Marc-Antoine Ruel <maruel@chromium.org> Date: Thu Jan 17 00:41:29 2019 swarming_bot: rewrite task_runner_test, task_runner send SIGTERM only once - task_runner: Do not send SIGTERM more than once, it's unnecessary. - task_runner_test.py previously used HTTP request level expectations via mocking, which had implicit timing assumptions. These timing assumptions (especially w.r.t. stdout buffering) didn't hold on Windows, which has a different clock resolution. Switching to a fake makes the tests work on Windows too. Manually verified. - Improve the fake Swarming bot API server implementation. - Skip IO timeout tests on Windows, since the currently flow doesn't guarantee cleanup. This also enables a more seamless migration to a pRPC based Bot API, so it's not all wasted time. Manually verified to pass on Windows and macOS. Bug: 921842 Change-Id: Ibcaad8b29d7f4c4fb8d16f9fe5d1a1e2ef4f9c86 Reviewed-on: https://chromium-review.googlesource.com/c/1412259 Commit-Queue: Marc-Antoine Ruel <maruel@chromium.org> Reviewed-by: Quinten Yearsley <qyearsley@chromium.org> Auto-Submit: Marc-Antoine Ruel <maruel@chromium.org> [modify] https://crrev.com/fac63cee56e1659cc3e8360f77603aaaac27c28b/appengine/swarming/swarming_bot/bot_code/task_runner.py [modify] https://crrev.com/fac63cee56e1659cc3e8360f77603aaaac27c28b/appengine/swarming/swarming_bot/bot_code/task_runner_test.py [modify] https://crrev.com/fac63cee56e1659cc3e8360f77603aaaac27c28b/appengine/swarming/swarming_bot/main_test.py [modify] https://crrev.com/fac63cee56e1659cc3e8360f77603aaaac27c28b/appengine/swarming/swarming_bot/swarmingserver_bot_fake.py
,
Jan 17
(6 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/infra/luci/luci-py.git/+/0c298134234fe93391db406a5a0d70f351248685 commit 0c298134234fe93391db406a5a0d70f351248685 Author: Marc-Antoine Ruel <maruel@chromium.org> Date: Thu Jan 17 01:11:05 2019 swarming_bot: disable buffering, fix kill_and_wait() - Disable buffering when calling run_isolated, this way it reduces the risk of latency when sending it to the bot. - Fix kill_and_wait() to work. It never worked. :( - Add a unit test to make sure this is handled properly, and manually tested on macOS and Windows. Bug: 921842 Change-Id: Ieaf3f4cd3346aebc4e3c9ff317d394d7e0a4a165 Reviewed-on: https://chromium-review.googlesource.com/c/1412253 Commit-Queue: Marc-Antoine Ruel <maruel@chromium.org> Auto-Submit: Marc-Antoine Ruel <maruel@chromium.org> Reviewed-by: Quinten Yearsley <qyearsley@chromium.org> [modify] https://crrev.com/0c298134234fe93391db406a5a0d70f351248685/appengine/swarming/swarming_bot/bot_code/task_runner.py [modify] https://crrev.com/0c298134234fe93391db406a5a0d70f351248685/appengine/swarming/swarming_bot/bot_code/task_runner_test.py
,
Jan 17
(6 days ago)
Took a bit more work than expected for it's finally fixed! The work done as part of this issue also helped with issue 913953 because the new server API fake will be easier to replace. |
||
►
Sign in to add a comment |
||
Comment 1 by bugdroid1@chromium.org
, Jan 15