[devicelab] Cocoon client#68333
Conversation
flutter-dashboard
bot
added
the
c: contributor-productivity
dev/devicelab/bin/run.dart
Outdated
| /// Key for the task to upload to in Flutter infrastructure. | ||
| /// | ||
| /// Only used if [serviceAccountFile] is passed. | ||
| String taskKey; |
There was a problem hiding this comment.
I think we have all the resources needed to compute the task key here, but then we make it harder to change it on the backend.
There was a problem hiding this comment.
This is a good point where are not going to have server generated keys anymore. As the task entry will be generated until cocoon gets the first notification from luci.
There was a problem hiding this comment.
Is it possible to only use the commit?
There was a problem hiding this comment.
We can pass the commit instead, and update cocoon to take the commit sha + task name.
There was a problem hiding this comment.
Can we assume git will be on the host running this test? If so, we could just pull this from git
There was a problem hiding this comment.
I started to work on the backend change for this, and realized we're going to need to send the branch as well.
Do you know if recipes knows about the branch?
There was a problem hiding this comment.
They now about the release ref dev, stable, etc. But I assume getting the branch is possible using git.
There was a problem hiding this comment.
I got a bit further in the backend, and we can scratch pulling the branch here. I removed the git commit sha flag, and pull it in the cocoon client via git.
If we switch to building these tests as a binary for the bot, we'll need to pass the commit sha as a flag.
jonahwilliams
left a comment
jonahwilliams
left a comment
There was a problem hiding this comment.
Nice! LGTM
Also: #68240
dev/devicelab/bin/run.dart
Outdated
| /// Key for the task to upload to in Flutter infrastructure. | ||
| /// | ||
| /// Only used if [serviceAccountFile] is passed. | ||
| String taskKey; |
There was a problem hiding this comment.
This is a good point where are not going to have server generated keys anymore. As the task entry will be generated until cocoon gets the first notification from luci.
| ); | ||
| } | ||
|
|
||
| /// Call [fn] retrying so long as [retryIf] return `true` for the exception |
There was a problem hiding this comment.
Can we use the retry package here instead of reimplementing?
There was a problem hiding this comment.
I had this discussion on discord, and consensus is for something trivial like this to just implement it. It prevents future headaches.
Initially, I did try it and kept running into headaches with getting it to not have pub issues.
dev/devicelab/bin/run.dart
Outdated
| /// Key for the task to upload to in Flutter infrastructure. | ||
| /// | ||
| /// Only used if [serviceAccountFile] is passed. | ||
| String taskKey; |
There was a problem hiding this comment.
Is it possible to only use the commit?
godofredoc
left a comment
godofredoc
left a comment
There was a problem hiding this comment.
Thanks for implementing this functionality.
4 participants
Description
Ports logic from flutter/cocoon agent to be directly included in the test runner script.
If
service-account-fileandtask-keyare passed on devicelab runs, it will attempt to upload the results to Cocoon.This is needed for migrating the devicelab to LUCI as we are deprecating the Cocoon agent.
Related Issues
#66191 - Add support to devicelab test runner to upload results to Cocoon
Tests
I added the following tests:
Cocoon checks for validating the sent request and reading the service account file correctly.
Runner invokes cocoon upload when a service account file flag is passed.
Checklist
Before you create this PR, confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Did any tests fail when you ran them? Please read [Handling breaking changes].