Migrate gpu integration tests to use Skia Gold for image diffing |
||||||||||||||||||||
Issue descriptionI file this as the bug to migrate gpu integration tests to use Skia Gold for image diffing. Reason is Skia Gold provide a very good UX to understand what the image diffs are (example: https://gold.skia.org/search?blame=2372a3352325bc45ebef8beafb5a9da8b6582f77&fdiffmax=-1&fref=false&frgbamax=255&frgbamin=0&head=true&include=false&limit=50&master=false&match=gamma_correct&match=name&metric=combined&neg=false&offset=0&pos=false&query=source_type%3Dgm&sort=desc&unt=true) ⛆ |
|
|
,
Aug 17
Whoever have bandwidth to take this, I guess :-). Team-wise, I think this for now owned by gpu team.
,
Aug 24
stephana@: I see from your internal writeup that Gold is close to being able to run on a commit queue. Is there a Skia bug I could block this on for reference?
,
Aug 28
,
Oct 1
,
Nov 13
Work-in-progress, joint work with kjlubick@ and stephana@: https://chromium-review.googlesource.com/1332733
,
Nov 13
,
Nov 13
With the current interest in possibly moving layout tests to a Skia Gold-based solution, plus a desire to have a unified image diff solution throughout Chromium (which is what prompted my initial attempt at getting Gold integration), it's looking like we might have some generic way of using Gold from Chromium in the nearish future. If that ends up being the case, is should there be anything preventing the GPU tests from using that instead of something like your current prototype that's GPU-specific?
,
Nov 13
Our team is happy to migrate to whatever the preferred way of invoking Gold is. The CL above is just an attempt to wire in stephana@'s work-in-progress goldctl command to the Chrome GPU tests as a proof of concept.
,
Nov 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2e75e2fafd62a2fa3df337b6ecfad8659ca7e41f commit 2e75e2fafd62a2fa3df337b6ecfad8659ca7e41f Author: Kenneth Russell <kbr@chromium.org> Date: Thu Nov 15 22:37:28 2018 Collapse symlinks when running "isolate.py remap". Without this, "mb.py zip" can't include CIPD dependencies, which are symlinks in the source tree. Uses kjlubick's earlier work from https://codereview.chromium.org/2844063005 . Bug: 850107 Change-Id: Id96b85946ee15bcb2edfcf002016fcaad7b1a98b Reviewed-on: https://chromium-review.googlesource.com/c/1338238 Reviewed-by: John Budorick <jbudorick@chromium.org> Commit-Queue: Kenneth Russell <kbr@chromium.org> Cr-Commit-Position: refs/heads/master@{#608556} [modify] https://crrev.com/2e75e2fafd62a2fa3df337b6ecfad8659ca7e41f/tools/mb/mb.py
,
Nov 19
Next step for me is to get goldctl properly into CIPD https://chromium-review.googlesource.com/c/infra/infra/+/1337542
,
Nov 26
kjlubick@: sounds good. Let me assign this to you for that task; please reassign back to me when we're ready to start testing the goldctl integration.
,
Dec 3
,
Dec 5
The following revision refers to this bug: https://chromium.googlesource.com/infra/infra/+/75424d8b0e2aec3bde407c03ea6fac3755ace81a commit 75424d8b0e2aec3bde407c03ea6fac3755ace81a Author: Kevin Lubick <kjlubick@google.com> Date: Wed Dec 05 20:16:48 2018 Add goldctl build integration Bug: 850107 Change-Id: I7a5a95ed7de8905c7e7b3211e362cca899ea09fc Reviewed-on: https://chromium-review.googlesource.com/c/1358928 Commit-Queue: Kevin Lubick <kjlubick@chromium.org> Reviewed-by: Robbie Iannucci <iannucci@chromium.org> Cr-Commit-Position: refs/heads/master@{#19368} [add] https://crrev.com/75424d8b0e2aec3bde407c03ea6fac3755ace81a/build/packages/goldctl.yaml [modify] https://crrev.com/75424d8b0e2aec3bde407c03ea6fac3755ace81a/go/deps.lock [modify] https://crrev.com/75424d8b0e2aec3bde407c03ea6fac3755ace81a/go/deps.yaml
,
Dec 7
goldctl is in CIPD the right way. Passing back to kbr@ to update the CL and try it out. To update goldctl, one must check out https://chromium.googlesource.com/infra/infra/ via a typical chrome infra checkout. Then, infra $ ./go/env.py # This will change your GOPATH etc for this terminal infra $ ./go/deps.py update # To test that it builds locally (and that the third_party deps of goldctl haven't been expanded) infra $ ./go/deps.py install infra $ go install go.skia.org/infra/gold-client/cmd/goldctl infra $ which goldctl # verify it's in infra/go/bin/ infra $ goldctl The deps.py update will have rolled the skia infra check out to ToT and the next time the builder runs, it will build a new version. Then update the git_version in the DEPS file to be whatever that new build is. Keep an eye on https://chrome-infra-packages.appspot.com/p/skia/tools/goldctl/linux-amd64/+/ to see which version if the infra repo (not the Skia infra repo, the Chrome infra repo) to use.
,
Dec 12
Taking this at kbr@'s suggestion. I'll need to do a bit of catch-up on the WIP CL and goldctl, but I think I should get to testing this locally tomorrow.
,
Dec 12
Thanks Brian for volunteering to help with this!
,
Dec 12
kjlubick@ and stephana@ - the WIP CL has the Gold instance set to "chrome-gpu", but chrome-gpu.skia.org just redirects to skia.org - was a Gold instance for this ever set up? If not, I can probably use the old chrome-vr-gold.skia.org that was setup a long time ago for local testing until the GPU team's instance is set up since it looks like it's still functional.
,
Dec 12
,
Dec 12
,
Dec 14
After only a small amount of tweaking (goldctl doesn't like non-string values in the keys file), it looks like the WIP CL is properly uploading to the bucket in both trybot and non-trybot modes. However, I'm a bit blocked on verifying that the test properly pass/fails because it always fails due to baselines not being produced yet. We'll have to wait for the gold instance to be set up for this to be properly fixed, but I should be able to do some more verification by uploading my own baselines to the bucket in the meantime. It doesn't look like goldctl actually outputs anything to the provided failure file, though.
,
Dec 14
The instance is up now and additional features have landed in goldctl, but both need another round of changes to really work. It's going to be until at least Monday before that lands.
,
Dec 14
-nednguyen@google.com account here since I filed this bug with @chromium account, and having both are doubling emails to my inbox from this thread :-(
,
Dec 17
,
Dec 17
,
Dec 17
,
Dec 17
,
Jan 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0e0f26dfea4a4dbfefbc5e775cc093dfcbe0c390 commit 0e0f26dfea4a4dbfefbc5e775cc093dfcbe0c390 Author: bsheedy <bsheedy@chromium.org> Date: Wed Jan 09 22:17:16 2019 Prototype integration of Skia Gold into Chrome GPU pixel tests. Updated version of crrev.com/c/1332733. This makes the following changes: 1. Adds the goldctl binary to DEPS via CIPD for interacting with Skia Gold instances. 2. Updates the GPU team's pixel integration test runner to support using Skia Gold for image comparison in addition to the legacy code path. To test, patch this in, run gclient sync, then invoke the following example to run as if on a trybot: ./content/test/gpu/run_gpu_integration_test.py pixel \ --browser=release --test-filter=Pixel_CSS3DBlueBox \ --use-skia-gold --build-revision f00ba3 \ --review-patch-issue 1234567 --review-patch-set 2 \ --buildbucket-build-id 800cdefg \ --download-refimg-from-cloud-storage --passthrough Or the following to run as if on a continuous bot: ./content/test/gpu/run_gpu_integration_test.py pixel \ --browser=release --test-filter=Pixel_CSS3DBlueBox \ --use-skia-gold --build-revision f00ba3 \ --upload-refimg-to-cloud-storage --passthrough Bug: 850107 Change-Id: I3b91e8e2d785e8da728ddcbc542db1eecf96d21a Reviewed-on: https://chromium-review.googlesource.com/c/1401234 Commit-Queue: Brian Sheedy <bsheedy@chromium.org> Reviewed-by: Kenneth Russell <kbr@chromium.org> Cr-Commit-Position: refs/heads/master@{#621328} [modify] https://crrev.com/0e0f26dfea4a4dbfefbc5e775cc093dfcbe0c390/DEPS [modify] https://crrev.com/0e0f26dfea4a4dbfefbc5e775cc093dfcbe0c390/chrome/test/BUILD.gn [modify] https://crrev.com/0e0f26dfea4a4dbfefbc5e775cc093dfcbe0c390/content/test/gpu/gpu_tests/cloud_storage_integration_test_base.py [modify] https://crrev.com/0e0f26dfea4a4dbfefbc5e775cc093dfcbe0c390/content/test/gpu/gpu_tests/pixel_integration_test.py
,
Jan 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2796783fdb6a9da2d4d667c68008459ce1268bd8 commit 2796783fdb6a9da2d4d667c68008459ce1268bd8 Author: Takuto Ikuta <tikuta@chromium.org> Date: Thu Jan 10 21:52:10 2019 Add /tools/skia_goldctl/ to .gitignore This is follow up for https://chromium-review.googlesource.com/c/chromium/src/+/1401234 Bug: 850107 Change-Id: I159a9810cc1fca1ff1ec9696b2e434f67d2ba538 Reviewed-on: https://chromium-review.googlesource.com/c/1404897 Reviewed-by: Brian Sheedy <bsheedy@chromium.org> Reviewed-by: Kenneth Russell <kbr@chromium.org> Commit-Queue: Takuto Ikuta <tikuta@chromium.org> Cr-Commit-Position: refs/heads/master@{#621756} [modify] https://crrev.com/2796783fdb6a9da2d4d667c68008459ce1268bd8/.gitignore
,
Jan 16
The following revision refers to this bug: https://chromium.googlesource.com/infra/infra/+/a2032fc6c4f9616c918bed5290fb2c074b383a76 commit a2032fc6c4f9616c918bed5290fb2c074b383a76 Author: bsheedy <bsheedy@chromium.org> Date: Wed Jan 16 02:17:21 2019 [go] Roll deps.lock Rolls deps.lock to pick up a newer Skia goldctl version. deps.py update output: Updated repos: https://code.googlesource.com/gocloud.git/+log/a93970efcf20..105f0564f8d6 https://chromium.googlesource.com/external/github.com/Microsoft/go-winio.git/+log/97e4973ce50b..6c8aa416ae89 https://chromium.googlesource.com/external/github.com/aws/aws-sdk-go.git/+log/29ae2fc88231..beaa15b1b227 https://chromium.googlesource.com/external/github.com/cenkalti/backoff.git/+log/62661b46c409..1e4cf3da5598 https://chromium.googlesource.com/external/github.com/chromedp/cdproto.git/+log/c09467c16a87..8e3eef48e6c8 https://chromium.googlesource.com/external/github.com/chromedp/chromedp.git/+log/98d4b0de6e1b..7f54f3f93c18 https://chromium.googlesource.com/external/github.com/disintegration/imaging.git/+log/9458da53d1e6..fa79ab36ab53 https://chromium.googlesource.com/external/github.com/golang/protobuf.git/+log/1d3f30b51784..347cf4a86c1c https://chromium.googlesource.com/external/github.com/googleapis/gax-go.git/+log/c8284fbb997c..ddfab93c3fae https://chromium.googlesource.com/external/github.com/gorilla/mux.git/+log/ef912dd76ebe..08e7f807d38d https://chromium.googlesource.com/external/github.com/klauspost/cpuid.git/+log/c640019ed4bd..0da02118eaa3 https://chromium.googlesource.com/external/github.com/pelletier/go-buffruneio.git/+log/de1592c34d9c..25c428535bd3 https://chromium.googlesource.com/external/github.com/pkg/errors.git/+log/ba968bfe8b2f..ffb6e22f0193 https://chromium.googlesource.com/external/github.com/prometheus/client_golang.git/+log/fb3d5cb2ad57..18d13eacc9cc https://chromium.googlesource.com/external/github.com/prometheus/client_model.git/+log/5c3871d89910..56726106282f https://chromium.googlesource.com/external/github.com/prometheus/common.git/+log/67670fe90761..b1c43a6df3ae https://chromium.googlesource.com/external/github.com/prometheus/procfs.git/+log/14fa7590c24d..b1a0a9a36d74 https://chromium.googlesource.com/external/github.com/spf13/cobra.git/+log/d2d81d9a96e2..7547e83b2d85 https://chromium.googlesource.com/external/github.com/stretchr/testify.git/+log/ffdc059bfe9c..363ebb24d041 https://chromium.googlesource.com/external/github.com/xeipuuv/gojsonschema.git/+log/ac52e6811b56..f971f3cd73b2 https://skia.googlesource.com/buildbot.git/+log/ae4a64ed878f..450365c3c6ba https://chromium.googlesource.com/external/github.com/google/starlark-go.git/+log/746fad3713e3..d37220ea0f19 https://go.googlesource.com/net.git/+log/927f97764cc3..915654e7eabc https://go.googlesource.com/oauth2.git/+log/d668ce993890..5dab4167f31c https://go.googlesource.com/tools.git/+log/8a6051197512..2e4132e53b93 https://code.googlesource.com/google-api-go-client.git/+log/19e022d8cf43..07f56c9e3ab5 https://chromium.googlesource.com/external/github.com/google/go-genproto.git/+log/bd9b4fb69e2f..db91494dd46c https://chromium.googlesource.com/external/github.com/grpc/grpc-go.git/+log/c71aa62423b3..a1ead1ef6c56 Bug: 850107 Change-Id: I84737b4eb9b5b612cf145e07c771c2a2bd2dd2eb Reviewed-on: https://chromium-review.googlesource.com/c/1413778 Reviewed-by: Vadim Shtayura <vadimsh@chromium.org> Commit-Queue: Brian Sheedy <bsheedy@chromium.org> Cr-Commit-Position: refs/heads/master@{#20012} [modify] https://crrev.com/a2032fc6c4f9616c918bed5290fb2c074b383a76/go/deps.lock |
|||||||||||||||||
►
Sign in to add a comment |
||||||||||||||||||||
Comment 1 by enne@chromium.org
, Aug 17