Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
25530003:
Rename load_test to isolateserver_load_test, create swarming_load_test. (Closed)
|
Created:
7 years, 2 months ago by M-A Ruel Modified:
7 years, 2 months ago CC:
chromium-reviews, csharp+cc_chromium.org, vadimsh+cc_chromium.org, maruel+cc_chromium.org Base URL:
https://chromium.googlesource.com/a/chromium/tools/swarm_client@2_exception Visibility:
Public. |
DescriptionCreate swarming load test.
swarming_load_test_bot.py creates fake Swarming bots that fake running tasks.
swarming_load_test_client.py creates fake Swarming tasks and wait for them to
complete.
The goal here is to see how the server melts down, if it does, when a lot of
concurrent pending tasks occur.
R=csharp@chromium.org
BUG=
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=227002
Patch Set 1 #Patch Set 2 : better #Patch Set 3 : Remove sig_handler #Patch Set 4 : Now works #Patch Set 5 : Remove broken code asserting progress.size #Patch Set 6 : Use named columns #
Total comments: 23
Patch Set 7 : Address review comments #
Total comments: 24
Patch Set 8 : Address comments #Patch Set 9 : Add unregister call #Patch Set 10 : Use Comodore64 instead of AIX, do not force bot to run on same host as client #Patch Set 11 : Rebase on HEAD #Patch Set 12 : This CL will get in one way or another #
Messages
Total messages: 21 (0 generated)
The CL is finally ready to go. It is dependent on: https://codereview.chromium.org/25478010 https://codereview.chromium.org/25478012 It does kill the canary swarming server deterministically, even if there is 200 fake swarm bot available, if the trigger rate is 16 tasks/s, ~20 bots get a task assigned, after that it's basically staling. I highly suspect transaction contention but I haven't verified. At least now we have a tool to do A/B testing of the server code to compare how the techniques fare in practice.
> It does kill the canary swarming server deterministically... Good job! :D Can we setup a separate swarming app engine instance for this sort of tests? I'd prefer canary be alive most of the time...
On 2013/10/02 17:58:49, Vadim Sh. wrote: > > It does kill the canary swarming server deterministically... > > Good job! :D Can we setup a separate swarming app engine instance for this sort > of tests? I'd prefer canary be alive most of the time... I don't think it would be worth the effort to setup an canary, since these tests shouldn't be too common (if they end up being too common then we probably have enough problems with swarm that this canary would probably not be too helpful). Plus, where is the fun in a canary if we can't willing break it every now and then :) M-A, can you ping me on this cl after https://codereview.chromium.org/25478010 and https://codereview.chromium.org/25478012 are submitted. I had just noticed a few breaks locally with all three patches together (mostly mismatched field names) and I'd rather review this once I know that has been fixed. Although if you still want me to review it now, just say so, although I'm not in a huge rush to see this get in since I was able to use it locally to verify the new server holds up much better.
On 2013/10/02 19:09:12, csharp wrote: > On 2013/10/02 17:58:49, Vadim Sh. wrote: > > > It does kill the canary swarming server deterministically... > > > > Good job! :D Can we setup a separate swarming app engine instance for this > sort > > of tests? I'd prefer canary be alive most of the time... > > I don't think it would be worth the effort to setup an canary, since these tests > shouldn't be too common (if they end up being too common then we probably have > enough problems with swarm that this canary would probably not be too helpful). > Plus, where is the fun in a canary if we can't willing break it every now and > then :) > > M-A, can you ping me on this cl after https://codereview.chromium.org/25478010 > and https://codereview.chromium.org/25478012 are submitted. I had just noticed a > few breaks locally with all three patches together (mostly mismatched field > names) and I'd rather review this once I know that has been fixed. > > Although if you still want me to review it now, just say so, although I'm not in > a huge rush to see this get in since I was able to use it locally to verify the > new server holds up much better. I agree with Chris, it's the canary purpose to be tested hard. No need to create yet another instance. When we kill it, it's only for a few tens of minutes anyway. Chris, this CL is ready for review.
> When we kill it, it's only for a few tens of minutes > anyway. For some reason last successful run on Linux canary slave was 5 hours ago. Doesn't look like 10 min to me...
Didn't get a chance to look at the other file yet but I sent it out now since I'll be at the (local) Windows conference for the rest of the week so I'm not sure when I'll next have time to look at this. https://codereview.chromium.org/25530003/diff/13001/tools/swarming_load_test_... File tools/swarming_load_test_bot.py (right): https://codereview.chromium.org/25530003/diff/13001/tools/swarming_load_test_... tools/swarming_load_test_bot.py:53: #print('Average overhead: %s' % graph.to_units(total_size / len(sizes))) Remove or uncomment https://codereview.chromium.org/25530003/diff/13001/tools/swarming_load_test_... tools/swarming_load_test_bot.py:55: if failures: Nit: new line above https://codereview.chromium.org/25530003/diff/13001/tools/swarming_load_test_... tools/swarming_load_test_bot.py:65: def calculate_version(url): Neat way of tricking the server https://codereview.chromium.org/25530003/diff/13001/tools/swarming_load_test_... tools/swarming_load_test_bot.py:89: If someones fires up a real AIX slave, well, sorry. But as the greatest OS ever, shouldn't this be a big concern? https://codereview.chromium.org/25530003/diff/13001/tools/swarming_load_test_... tools/swarming_load_test_bot.py:143: # Insert real code to fetch task from Swarming here. I'm a bit confused by this comment, it kind of reads like a todo (which I assume it isn't). Maybe just drop the "Insert real code to" https://codereview.chromium.org/25530003/diff/13001/tools/swarming_load_test_... tools/swarming_load_test_bot.py:148: # Work around an internal bug in urllib3. If this is an internal urllib3 bug why are be storing it in the events? Can't we just ignore it? https://codereview.chromium.org/25530003/diff/13001/tools/swarming_load_test_... tools/swarming_load_test_bot.py:166: time.sleep(manifest['come_back']) Why we do we follow this parameter when we ignore the ping delay one? (I think it is fine to do it this way, I'm just wondering why) https://codereview.chromium.org/25530003/diff/13001/tools/swarming_load_test_... tools/swarming_load_test_bot.py:169: if commands == ['UpdateSlave']: Why not just call this once before creating all the fake bots, them we can just initialize them with the correct version instead of having each one of them "update" https://codereview.chromium.org/25530003/diff/13001/tools/swarming_load_test_... tools/swarming_load_test_bot.py:189: assert manifest['commands'][1] == { I'm not sure we should be checking that the exact commands matches, I think just checking it contains local_test_runner is probably enough. https://codereview.chromium.org/25530003/diff/13001/tools/swarming_load_test_... tools/swarming_load_test_bot.py:203: while True: Since we know how many times we should execute this loop, why not use a for-loop instead? https://codereview.chromium.org/25530003/diff/13001/tools/swarming_load_test_... tools/swarming_load_test_bot.py:219: 'result_output': 'This task ran with great success', Excellent output :)
https://codereview.chromium.org/25530003/diff/13001/tools/swarming_load_test_... File tools/swarming_load_test_bot.py (right): https://codereview.chromium.org/25530003/diff/13001/tools/swarming_load_test_... tools/swarming_load_test_bot.py:53: #print('Average overhead: %s' % graph.to_units(total_size / len(sizes))) On 2013/10/02 21:56:24, csharp wrote: > Remove or uncomment Done. https://codereview.chromium.org/25530003/diff/13001/tools/swarming_load_test_... tools/swarming_load_test_bot.py:55: if failures: On 2013/10/02 21:56:24, csharp wrote: > Nit: new line above Done. https://codereview.chromium.org/25530003/diff/13001/tools/swarming_load_test_... tools/swarming_load_test_bot.py:89: If someones fires up a real AIX slave, well, sorry. On 2013/10/02 21:56:24, csharp wrote: > But as the greatest OS ever, shouldn't this be a big concern? We can fix that once it becomes a problem. I could rename to Comodore64 if you prefer, since there's not python for Comodore. https://codereview.chromium.org/25530003/diff/13001/tools/swarming_load_test_... tools/swarming_load_test_bot.py:143: # Insert real code to fetch task from Swarming here. On 2013/10/02 21:56:24, csharp wrote: > I'm a bit confused by this comment, it kind of reads like a todo (which I assume > it isn't). Maybe just drop the "Insert real code to" It was a todo that I forgot, removed. https://codereview.chromium.org/25530003/diff/13001/tools/swarming_load_test_... tools/swarming_load_test_bot.py:148: # Work around an internal bug in urllib3. On 2013/10/02 21:56:24, csharp wrote: > If this is an internal urllib3 bug why are be storing it in the events? Can't we > just ignore it? The upgrade to requests v2 should have fixed it, I removed this handler since I don't recall seeing it since Vadim did the upgrade. https://codereview.chromium.org/25530003/diff/13001/tools/swarming_load_test_... tools/swarming_load_test_bot.py:166: time.sleep(manifest['come_back']) On 2013/10/02 21:56:24, csharp wrote: > Why we do we follow this parameter when we ignore the ping delay one? (I think > it is fine to do it this way, I'm just wondering why) No specific reason. What do you think is best? https://codereview.chromium.org/25530003/diff/13001/tools/swarming_load_test_... tools/swarming_load_test_bot.py:169: if commands == ['UpdateSlave']: On 2013/10/02 21:56:24, csharp wrote: > Why not just call this once before creating all the fake bots, them we can just > initialize them with the correct version instead of having each one of them > "update" Done. https://codereview.chromium.org/25530003/diff/13001/tools/swarming_load_test_... tools/swarming_load_test_bot.py:203: while True: On 2013/10/02 21:56:24, csharp wrote: > Since we know how many times we should execute this loop, why not use a for-loop > instead? Because the call net.url_read() takes a indeterminate amount of time.
https://codereview.chromium.org/25530003/diff/13001/tools/swarming_load_test_... File tools/swarming_load_test_bot.py (right): https://codereview.chromium.org/25530003/diff/13001/tools/swarming_load_test_... tools/swarming_load_test_bot.py:166: time.sleep(manifest['come_back']) On 2013/10/02 23:44:25, M-A Ruel wrote: > On 2013/10/02 21:56:24, csharp wrote: > > Why we do we follow this parameter when we ignore the ping delay one? (I think > > it is fine to do it this way, I'm just wondering why) > > No specific reason. What do you think is best? After thinking about this over night, I actually think we should obey the swarm server and wait the correct amount of time before polling again or pinging. I figure that the swarm server can adjust those values to try and reduce its load, so the load test should respect those values. (although atm the ping isn't adjusted based on load, but that might be an interesting future idea). https://codereview.chromium.org/25530003/diff/13001/tools/swarming_load_test_... tools/swarming_load_test_bot.py:189: assert manifest['commands'][1] == { On 2013/10/02 21:56:24, csharp wrote: > I'm not sure we should be checking that the exact commands matches, I think just > checking it contains local_test_runner is probably enough. ping? https://codereview.chromium.org/25530003/diff/18001/tools/swarming_load_test_... File tools/swarming_load_test_bot.py (right): https://codereview.chromium.org/25530003/diff/18001/tools/swarming_load_test_... tools/swarming_load_test_bot.py:161: if commands == ['UpdateSlave']: I don't think this is needed anymore, right? https://codereview.chromium.org/25530003/diff/18001/tools/swarming_load_test_... tools/swarming_load_test_bot.py:244: group.add_option( As mentioned before, probably remove this since pings can be viewed as the servers way of controlling flow (we can always just increase the bot # for the same effect) https://codereview.chromium.org/25530003/diff/18001/tools/swarming_load_test_... tools/swarming_load_test_bot.py:278: swarm_bot_hash = calculate_version(options.swarming + '/get_slave_code') Nit: Move to just above line 287 https://codereview.chromium.org/25530003/diff/18001/tools/swarming_load_test_... tools/swarming_load_test_bot.py:301: # The slaves could be told to suicide. Nit: suicide-> die ("be told to suicide" just sounds a bit weird) https://codereview.chromium.org/25530003/diff/18001/tools/swarming_load_test_... tools/swarming_load_test_bot.py:306: progress.update_item('Waiting for slaves to quit.', raw=True) It would be nice it when the fake bots went down they could also remove themselves from the server's machine list (I just realized it it filled with fake bots atm) https://codereview.chromium.org/25530003/diff/18001/tools/swarming_load_test_... File tools/swarming_load_test_client.py (right): https://codereview.chromium.org/25530003/diff/18001/tools/swarming_load_test_... tools/swarming_load_test_client.py:69: response = net.url_open(swarming_url + '/test', data=data) I encountered problems with this locally when I accidentally typed http:// instead of https://. The redirect seemed to cause problems because the redirect was a GET instead of a POST (so swarm rejected it). I didn't dig too deeply into what code in this script couldn't handle that, but it didn't give me a nice error message so I was confused for a bit. https://codereview.chromium.org/25530003/diff/18001/tools/swarming_load_test_... tools/swarming_load_test_client.py:109: u'output': u'This task ran with great success', Could we move this into a constant shared by the two scripts so they don't need to redefine it. https://codereview.chromium.org/25530003/diff/18001/tools/swarming_load_test_... tools/swarming_load_test_client.py:187: should_have_triggered = int(duration * options.send_rate) Nit: should_have_triggered -> should_have_triggered_so_far (It took me a bit to figure out how this worked to spread out the tests) https://codereview.chromium.org/25530003/diff/18001/utils/threading_utils.py File utils/threading_utils.py (right): https://codereview.chromium.org/25530003/diff/18001/utils/threading_utils.py#... utils/threading_utils.py:400: """Queue information to print out. Isn't this updating as well as queuing? https://codereview.chromium.org/25530003/diff/18001/utils/threading_utils.py#... utils/threading_utils.py:404: <name>: argument name is a name of a column. it's value is the increment I'm a bit confused by this comment, it seems like combination of explaining the kwargs inputs and the named name input, is that correct?
https://codereview.chromium.org/25530003/diff/13001/tools/swarming_load_test_... File tools/swarming_load_test_bot.py (right): https://codereview.chromium.org/25530003/diff/13001/tools/swarming_load_test_... tools/swarming_load_test_bot.py:166: time.sleep(manifest['come_back']) On 2013/10/03 16:08:04, csharp wrote: > After thinking about this over night, I actually think we should obey the swarm > server and wait the correct amount of time before polling again or pinging. > > I figure that the swarm server can adjust those values to try and reduce its > load, so the load test should respect those values. (although atm the ping isn't > adjusted based on load, but that might be an interesting future idea). Done. https://codereview.chromium.org/25530003/diff/13001/tools/swarming_load_test_... tools/swarming_load_test_bot.py:189: assert manifest['commands'][1] == { On 2013/10/03 16:08:04, csharp wrote: > On 2013/10/02 21:56:24, csharp wrote: > > I'm not sure we should be checking that the exact commands matches, I think > just > > checking it contains local_test_runner is probably enough. > > ping? Done. https://codereview.chromium.org/25530003/diff/18001/tools/swarming_load_test_... File tools/swarming_load_test_bot.py (right): https://codereview.chromium.org/25530003/diff/18001/tools/swarming_load_test_... tools/swarming_load_test_bot.py:161: if commands == ['UpdateSlave']: On 2013/10/03 16:08:04, csharp wrote: > I don't think this is needed anymore, right? It is, see line 163. https://codereview.chromium.org/25530003/diff/18001/tools/swarming_load_test_... tools/swarming_load_test_bot.py:244: group.add_option( On 2013/10/03 16:08:04, csharp wrote: > As mentioned before, probably remove this since pings can be viewed as the > servers way of controlling flow (we can always just increase the bot # for the > same effect) Done. https://codereview.chromium.org/25530003/diff/18001/tools/swarming_load_test_... tools/swarming_load_test_bot.py:278: swarm_bot_hash = calculate_version(options.swarming + '/get_slave_code') On 2013/10/03 16:08:04, csharp wrote: > Nit: Move to just above line 287 Done. https://codereview.chromium.org/25530003/diff/18001/tools/swarming_load_test_... tools/swarming_load_test_bot.py:301: # The slaves could be told to suicide. On 2013/10/03 16:08:04, csharp wrote: > Nit: suicide-> die ("be told to suicide" just sounds a bit weird) Done. https://codereview.chromium.org/25530003/diff/18001/tools/swarming_load_test_... tools/swarming_load_test_bot.py:306: progress.update_item('Waiting for slaves to quit.', raw=True) On 2013/10/03 16:08:04, csharp wrote: > It would be nice it when the fake bots went down they could also remove > themselves from the server's machine list (I just realized it it filled with > fake bots atm) Yep. I'd want this functionality for the Android swarming slaves too. Do you think it'd be hard to implement something, where only the "slave itself" could delete itself. Not sure how to do this. https://codereview.chromium.org/25530003/diff/18001/tools/swarming_load_test_... File tools/swarming_load_test_client.py (right): https://codereview.chromium.org/25530003/diff/18001/tools/swarming_load_test_... tools/swarming_load_test_client.py:109: u'output': u'This task ran with great success', On 2013/10/03 16:08:04, csharp wrote: > Could we move this into a constant shared by the two scripts so they don't need > to redefine it. Done. https://codereview.chromium.org/25530003/diff/18001/tools/swarming_load_test_... tools/swarming_load_test_client.py:187: should_have_triggered = int(duration * options.send_rate) On 2013/10/03 16:08:04, csharp wrote: > Nit: should_have_triggered -> should_have_triggered_so_far > (It took me a bit to figure out how this worked to spread out the tests) Done. https://codereview.chromium.org/25530003/diff/18001/utils/threading_utils.py File utils/threading_utils.py (right): https://codereview.chromium.org/25530003/diff/18001/utils/threading_utils.py#... utils/threading_utils.py:400: """Queue information to print out. On 2013/10/03 16:08:04, csharp wrote: > Isn't this updating as well as queuing? No, the function is illnamed. Note that this code is already committed. https://codereview.chromium.org/25530003/diff/18001/utils/threading_utils.py#... utils/threading_utils.py:404: <name>: argument name is a name of a column. it's value is the increment On 2013/10/03 16:08:04, csharp wrote: > I'm a bit confused by this comment, it seems like combination of explaining the > kwargs inputs and the named name input, is that correct? No, only kwargs. I had forgot to document 'name', doing.
lgtm assuming you add a todo to call delete_machine_stats or just start calling it (even though right now it will fail) https://codereview.chromium.org/25530003/diff/18001/tools/swarming_load_test_... File tools/swarming_load_test_bot.py (right): https://codereview.chromium.org/25530003/diff/18001/tools/swarming_load_test_... tools/swarming_load_test_bot.py:161: if commands == ['UpdateSlave']: On 2013/10/03 16:47:11, Marc-Antoine Ruel (Google) wrote: > On 2013/10/03 16:08:04, csharp wrote: > > I don't think this is needed anymore, right? > > It is, see line 163. I think we can just mark UpdateSlave as an unexpected command, we shouldn't be updating while doing a load test. https://codereview.chromium.org/25530003/diff/18001/tools/swarming_load_test_... tools/swarming_load_test_bot.py:306: progress.update_item('Waiting for slaves to quit.', raw=True) On 2013/10/03 16:47:11, Marc-Antoine Ruel (Google) wrote: > On 2013/10/03 16:08:04, csharp wrote: > > It would be nice it when the fake bots went down they could also remove > > themselves from the server's machine list (I just realized it it filled with > > fake bots atm) > > Yep. I'd want this functionality for the Android swarming slaves too. Do you > think it'd be hard to implement something, where only the "slave itself" could > delete itself. Not sure how to do this. The machine just needs to call: "/delete_machine_stats?r=%MACHINE_ID%" Currently the url is prefixed with /secure/, so machines can't do it yet, but I'll send out a cl to allow an user or machine to deregister a machine.
https://codereview.chromium.org/25530003/diff/18001/tools/swarming_load_test_... File tools/swarming_load_test_bot.py (right): https://codereview.chromium.org/25530003/diff/18001/tools/swarming_load_test_... tools/swarming_load_test_bot.py:161: if commands == ['UpdateSlave']: On 2013/10/03 19:00:34, csharp wrote: > On 2013/10/03 16:47:11, Marc-Antoine Ruel (Google) wrote: > > On 2013/10/03 16:08:04, csharp wrote: > > > I don't think this is needed anymore, right? > > > > It is, see line 163. > > I think we can just mark UpdateSlave as an unexpected command, we shouldn't be > updating while doing a load test. I disagree, this is something we should test, so that we can confirm AppEngine can ramp up fast when we switch between versions under heavy load. It's going to happen in production one day or another. https://codereview.chromium.org/25530003/diff/18001/tools/swarming_load_test_... tools/swarming_load_test_bot.py:306: progress.update_item('Waiting for slaves to quit.', raw=True) On 2013/10/03 19:00:34, csharp wrote: > On 2013/10/03 16:47:11, Marc-Antoine Ruel (Google) wrote: > > On 2013/10/03 16:08:04, csharp wrote: > > > It would be nice it when the fake bots went down they could also remove > > > themselves from the server's machine list (I just realized it it filled with > > > fake bots atm) > > > > Yep. I'd want this functionality for the Android swarming slaves too. Do you > > think it'd be hard to implement something, where only the "slave itself" could > > delete itself. Not sure how to do this. > > The machine just needs to call: "/delete_machine_stats?r=%MACHINE_ID%" > > Currently the url is prefixed with /secure/, so machines can't do it yet, but > I'll send out a cl to allow an user or machine to deregister a machine. > Done.
https://codereview.chromium.org/25530003/diff/18001/tools/swarming_load_test_... File tools/swarming_load_test_bot.py (right): https://codereview.chromium.org/25530003/diff/18001/tools/swarming_load_test_... tools/swarming_load_test_bot.py:161: if commands == ['UpdateSlave']: On 2013/10/03 19:15:29, Marc-Antoine Ruel (Google) wrote: > On 2013/10/03 19:00:34, csharp wrote: > > On 2013/10/03 16:47:11, Marc-Antoine Ruel (Google) wrote: > > > On 2013/10/03 16:08:04, csharp wrote: > > > > I don't think this is needed anymore, right? > > > > > > It is, see line 163. > > > > I think we can just mark UpdateSlave as an unexpected command, we shouldn't be > > updating while doing a load test. > > I disagree, this is something we should test, so that we can confirm AppEngine > can ramp up fast when we switch between versions under heavy load. > > It's going to happen in production one day or another. Ok.
On 2013/10/04 13:43:13, csharp wrote: > https://codereview.chromium.org/25530003/diff/18001/tools/swarming_load_test_... > File tools/swarming_load_test_bot.py (right): > > https://codereview.chromium.org/25530003/diff/18001/tools/swarming_load_test_... > tools/swarming_load_test_bot.py:161: if commands == ['UpdateSlave']: > On 2013/10/03 19:15:29, Marc-Antoine Ruel (Google) wrote: > > On 2013/10/03 19:00:34, csharp wrote: > > > On 2013/10/03 16:47:11, Marc-Antoine Ruel (Google) wrote: > > > > On 2013/10/03 16:08:04, csharp wrote: > > > > > I don't think this is needed anymore, right? > > > > > > > > It is, see line 163. > > > > > > I think we can just mark UpdateSlave as an unexpected command, we shouldn't > be > > > updating while doing a load test. > > > > I disagree, this is something we should test, so that we can confirm AppEngine > > can ramp up fast when we switch between versions under heavy load. > > > > It's going to happen in production one day or another. > > Ok. Note: In patchset 10, I switched from AIX (improbable) to Comodore64 (impossible). I also removed the check in _client for the bot to run on the same host, it is not useful.
On 2013/10/04 13:56:12, Marc-Antoine Ruel (Google) wrote: > On 2013/10/04 13:43:13, csharp wrote: > > > https://codereview.chromium.org/25530003/diff/18001/tools/swarming_load_test_... > > File tools/swarming_load_test_bot.py (right): > > > > > https://codereview.chromium.org/25530003/diff/18001/tools/swarming_load_test_... > > tools/swarming_load_test_bot.py:161: if commands == ['UpdateSlave']: > > On 2013/10/03 19:15:29, Marc-Antoine Ruel (Google) wrote: > > > On 2013/10/03 19:00:34, csharp wrote: > > > > On 2013/10/03 16:47:11, Marc-Antoine Ruel (Google) wrote: > > > > > On 2013/10/03 16:08:04, csharp wrote: > > > > > > I don't think this is needed anymore, right? > > > > > > > > > > It is, see line 163. > > > > > > > > I think we can just mark UpdateSlave as an unexpected command, we > shouldn't > > be > > > > updating while doing a load test. > > > > > > I disagree, this is something we should test, so that we can confirm > AppEngine > > > can ramp up fast when we switch between versions under heavy load. > > > > > > It's going to happen in production one day or another. > > > > Ok. > > Note: In patchset 10, I switched from AIX (improbable) to Comodore64 > (impossible). > I also removed the check in _client for the bot to run on the same host, it is > not useful. Still lgtm
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/maruel@chromium.org/25530003/6002
Failed to apply patch for swarm_client/googletest/run_test_cases.py:
While running patch -p1 --forward --force --no-backup-if-mismatch;
patching file swarm_client/googletest/run_test_cases.py
Hunk #1 FAILED at 1182.
1 out of 1 hunk FAILED -- saving rejects to file
swarm_client/googletest/run_test_cases.py.rej
Patch: swarm_client/googletest/run_test_cases.py
Index: googletest/run_test_cases.py
diff --git swarm_client/googletest/run_test_cases.py
swarm_client/googletest/run_test_cases.py
index
adddef4d83b11506b0686d86de2a4da0372e60e3..8139a9e10bc117b18254aa84028b2768b4cb9638
100755
--- a/swarm_client/googletest/run_test_cases.py
+++ b/swarm_client/googletest/run_test_cases.py
@@ -1182,10 +1182,10 @@ def run_test_cases(
if gtest_output:
gtest_output = gen_gtest_output_dir(cwd, gtest_output)
- progress = threading_utils.Progress(len(test_cases))
+ columns = [('index', 0), ('size', len(test_cases))]
+ progress = threading_utils.Progress(columns)
progress.use_cr_only = not no_cr
- serial_tasks = threading_utils.QueueWithProgress(0)
- serial_tasks.set_progress(progress)
+ serial_tasks = threading_utils.QueueWithProgress(progress)
def add_serial_task(priority, func, *args, **kwargs):
"""Adds a serial task, to be executed later."""
On 2013/10/04 14:06:33, I haz the power (commit-bot) wrote: > Failed to apply patch for swarm_client/googletest/run_test_cases.py: Sorry I had failed to rebase properly.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/maruel@chromium.org/25530003/45001
Presubmit check for 25530003-45001 failed and returned exit status 1. Running presubmit commit checks ... ** Presubmit Warnings ** Found lines longer than 80 characters (first 5 shown). swarm_client/tools/swarming_load_test_bot.py, line 111, 122 chars Presubmit checks took 76.6s to calculate. Was the presubmit check useful? Please send feedback & hate mail to maruel@chromium.org!
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/maruel@chromium.org/25530003/14003
Message was sent while issue was closed.
Change committed as 227002 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
