New issue
Advanced search Search tips

Issue 739119 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

[WPT import] Importer incorrectly considers CQ as failed if any jobs failed (e.g. due to flakiness)

Project Member Reported by foolip@chromium.org, Jul 4 2017

Issue description

#262 was because:
Traceback (most recent call last):
  File "/mnt/data/b/rr/tmpZRREnv/w/src/third_party/WebKit/Tools/Scripts/wpt-import", line 18, in <module>
    host.exit(importer.main())
  File "/mnt/data/b/rr/tmpZRREnv/w/src/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_importer.py", line 112, in main
    commit_successful = self.do_auto_update()
  File "/mnt/data/b/rr/tmpZRREnv/w/src/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_importer.py", line 363, in do_auto_update
    self.git_cl.run(['cl', 'upload', '--send-mail'])  # Turn off WIP mode.
  File "/mnt/data/b/rr/tmpZRREnv/w/src/third_party/WebKit/Tools/Scripts/webkitpy/common/net/git_cl.py", line 51, in run
    return self._host.executive.run_command(command, cwd=self._cwd)
  File "/mnt/data/b/rr/tmpZRREnv/w/src/third_party/WebKit/Tools/Scripts/webkitpy/common/system/executive.py", line 339, in run_command
    (error_handler or self.default_error_handler)(script_error)
  File "/mnt/data/b/rr/tmpZRREnv/w/src/third_party/WebKit/Tools/Scripts/webkitpy/common/system/executive.py", line 247, in default_error_handler
    raise error
webkitpy.common.system.executive.ScriptError: Failed to run "['git', 'cl', 'cl', 'upload', '--send-mail']" exit_code: 2
output: Usage: git cl <command> [options]
git cl: error: no such option: --send-mail

That's an easy fix I think.

#263, however, says "CQ appears to have failed", but the dry run did pass. qyearsley@, is it because there was one bot failure which was a success on retry?
Status: Available (was: Untriaged)
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d563ab82c0847587f6c6f797772841de08f3c185

commit d563ab82c0847587f6c6f797772841de08f3c185
Author: Philip Jägenstedt <foolip@chromium.org>
Date: Tue Jul 04 12:23:24 2017

Fix a `git cl cl upload --send-mail` typo wpt importer

This is from https://chromium-review.googlesource.com/550756.

Because of this typo, a recent automatic import failed.

Bug:  739119 
Change-Id: Icef99885c8e0d6d652bd0b97a194d2aa4905dee1
Reviewed-on: https://chromium-review.googlesource.com/558982
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Commit-Queue: Philip Jägenstedt <foolip@chromium.org>
Cr-Commit-Position: refs/heads/master@{#484091}
[modify] https://crrev.com/d563ab82c0847587f6c6f797772841de08f3c185/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_importer.py

Owner: qyears...@chromium.org
Status: Assigned (was: Available)
That takes care of the typo. qyearsley@, can you investigate #263, and route as appropriate? I think the problem is with `all(s == TryJobStatus('COMPLETED', 'SUCCESS') for _, s in try_results.iteritems())`. What would it take to use the same notion of passing the dry run as a human does?

There is an increased chance of introducing flakiness, but as long as we're attempting 4 imports per day, it seems like flaky tests would get committed eventually anyway. Can anything be done to mitigate that problem in turn?
Cc: -qyears...@chromium.org
Yep! test_importer should instead use GitCL.latest_try_jobs to check whether the try jobs were successful -- that method returns only the latest results for each builder, which is what we want in this case.
Summary: [WPT import] Importer incorrectly considers CQ as failed if any jobs failed (e.g. due to flakiness) (was: [WPT import] CLs with successful dry runs have been abandoned)
Project Member

Comment 7 by bugdroid1@chromium.org, Jul 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6cdda3e630081ea7c0fb25dcd0a98fcae0568925

commit 6cdda3e630081ea7c0fb25dcd0a98fcae0568925
Author: Quinten Yearsley <qyearsley@google.com>
Date: Thu Jul 20 22:39:52 2017

In wpt-importer, look only at the latest CQ job results.

Previously, the importer would sometimes abandon a potential import
even when it passed CQ if there were any failing (flaky) jobs.

This CL should make it so that when deciding whether the CQ passed,
importer looks only at the latest builds for each builder.

Bug:  739119 
Change-Id: I126e33191ddc24797501f97ff0834e3f9e45d4bc
Reviewed-on: https://chromium-review.googlesource.com/578141
Commit-Queue: Quinten Yearsley <qyearsley@chromium.org>
Reviewed-by: Jeff Carpenter <jeffcarp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488453}
[modify] https://crrev.com/6cdda3e630081ea7c0fb25dcd0a98fcae0568925/third_party/WebKit/Tools/Scripts/webkitpy/common/net/git_cl.py
[modify] https://crrev.com/6cdda3e630081ea7c0fb25dcd0a98fcae0568925/third_party/WebKit/Tools/Scripts/webkitpy/common/net/git_cl_mock.py
[modify] https://crrev.com/6cdda3e630081ea7c0fb25dcd0a98fcae0568925/third_party/WebKit/Tools/Scripts/webkitpy/common/net/git_cl_unittest.py
[modify] https://crrev.com/6cdda3e630081ea7c0fb25dcd0a98fcae0568925/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_importer.py
[modify] https://crrev.com/6cdda3e630081ea7c0fb25dcd0a98fcae0568925/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_importer_unittest.py

Status: Fixed (was: Assigned)
Should be fixed after that change.

Sign in to add a comment