Another example here: https://chromium-review.googlesource.com/c/609136

In this case, there was a failure when rebasing the patch and resubmitting it as far as I can see.

Regarding purple jobs: Sometimes in the past I've interrupted jobs that I know will fail if I want to retry immediately. I think that all of the purple wpt-importer jobs are jobs I interrupted.

For https://chromium-review.googlesource.com/c/609136: I'm assuming you mean rebaselining and uploading a new patchset. Not really sure what happened there.

In general in https://cs.chromium.org/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_importer.py, we run `git cl set-close` to try to abandon the CL when we know that it failed. If the import job finishes without running that, then the CL will remain open. It seems like this usually happens when there's some exception raised in the code.

Proposal: In TestImporter.main, we should catch exceptions and run `git cl set-close` before exiting.

> Proposal: In TestImporter.main, we should catch exceptions and run `git cl set-close` before exiting.

Does it make more sense to do that from test_importer.py or via the recipe itself (i.e. if the import step failed in any way run a step that calls `git cl set-close')?

Maybe as its first step, the importer could find and close all open reviews, without regard for when they were created or why they're still open?

Re #3: Doing it from the recipe sounds alright and might catch more cases than in test_importer.py, although doing it from test_importer.py would makes sense to me since then all of the calls to "git cl" and all of the unit tests would be in one place.

Re #4: That would be possible, although it may not be worth the extra complexity; unlike the exporter, the importer is currently not interacting with Gerrit directly at all.

I'm more inclined to catching the exception in test_importer.py and closing the CL before exiting, because:

1. Keep the code base cleaner and each component more self-contained, which means: a) no direct "git cl" invocation from the recipe; b) no Gerrit API calls in the importer.
2. Cleaning up previous runs at the beginning of each run doesn't sound like a good idea to me: a) it is a potential race condition if the previous run takes too long (which, I understand, is unlikely currently due to the timeout and intervals); b) there are legitimate cases where we manually restore abandoned import CLs, and it would be annoying to that person if the CL was suddenly closed by a cron job.
3. If there is anything we miss to catch (this approach will likely catch fewer cases than the other two proposed approaches), it is not hard to notice by looking at open CLs of blink-wpt-bot, and can be traced back to the importer job via CL description.

 Issue 797748  has been merged into this issue.

This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

robertma@, I guess this is still valid?