Speed-up the FilesApp open/close Quick View browser tests |
||||||
Issue descriptionThe tests test opening and opening+closing the quick view when a the space key is pressed on a selected file. The selected file is a large PNG image that is decoded and rescaled during these tests to display the result in quick view. Might be faster to select a simpler file (like a txt file) in these tests, since they only seem to test whether Files App can open / close the Quick View.
,
Jun 12 2018
,
Jun 13 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/438e9f0679fa1ac6889991714d9aa4dff1034d6a commit 438e9f0679fa1ac6889991714d9aa4dff1034d6a Author: Noel Gordon <noel@chromium.org> Date: Wed Jun 13 02:51:12 2018 Speed-up QuickView openQuickView browser test The test tests that Quick View opens by the usual method: user presses the space key on a selected file. Change the test to use a text file, rather than a PNG image, to remove image decode and rescale costs from the test-time. Also put all tests steps in a StepRunner group, so that all test steps can be debugged using the step runner browser test flag. Document each of the steps. Bug: 851888 Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation Change-Id: Ief21de93e9496f463506d2ae22ab28db76c087ef Reviewed-on: https://chromium-review.googlesource.com/1097041 Reviewed-by: Luciano Pacheco <lucmult@chromium.org> Commit-Queue: Noel Gordon <noel@chromium.org> Cr-Commit-Position: refs/heads/master@{#566699} [modify] https://crrev.com/438e9f0679fa1ac6889991714d9aa4dff1034d6a/ui/file_manager/integration_tests/file_manager/quick_view.js
,
Jun 13 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c0aa77df4da9f1eb1f788a4f04b0cb078e2903ef commit c0aa77df4da9f1eb1f788a4f04b0cb078e2903ef Author: Noel Gordon <noel@chromium.org> Date: Wed Jun 13 02:55:44 2018 Speed-up QuickView closeQuickView browser test The test tests that Quick View closes when users click Quick View when it is in open state. Change the test to use a text file, rather than a PNG image, to remove image decode and rescale costs from the test-time. The test awaits the close using a pending() call, but did not define a |caller|, which is a required argument of pending(). Fix that. Also put all tests steps in a StepRunner group, so that all test steps can be debugged using the step runner browser test flag. Document each of the test steps. Bug: 851888 Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation Change-Id: I39310b57f9ed09e5d164b95b11b7494dc45adffa Reviewed-on: https://chromium-review.googlesource.com/1097048 Commit-Queue: Noel Gordon <noel@chromium.org> Reviewed-by: Luciano Pacheco <lucmult@chromium.org> Cr-Commit-Position: refs/heads/master@{#566700} [modify] https://crrev.com/c0aa77df4da9f1eb1f788a4f04b0cb078e2903ef/ui/file_manager/integration_tests/file_manager/quick_view.js
,
Jun 13 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2111e0cf2eec22a80d0492eeb121686d281c98e5 commit 2111e0cf2eec22a80d0492eeb121686d281c98e5 Author: Noel Gordon <noel@chromium.org> Date: Wed Jun 13 08:47:35 2018 Increase QuickView browser test coverage With more speed, add more coverage: run open Quick View case in Tablet mode, and in Guest mode (so easy). Add a Quick View open test case for an ephemeral volume (USB here). Tablet (aka fully immersive) mode is not supported in Mash: add a Mash bot exclusion for the Tablet mode case. Bug: 851888 Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation Change-Id: I893f35838731b3264268b027a00a86c339bf5b3f Reviewed-on: https://chromium-review.googlesource.com/1098432 Commit-Queue: Noel Gordon <noel@chromium.org> Reviewed-by: Luciano Pacheco <lucmult@chromium.org> Cr-Commit-Position: refs/heads/master@{#566761} [modify] https://crrev.com/2111e0cf2eec22a80d0492eeb121686d281c98e5/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc [modify] https://crrev.com/2111e0cf2eec22a80d0492eeb121686d281c98e5/testing/buildbot/filters/mash.browser_tests.filter [modify] https://crrev.com/2111e0cf2eec22a80d0492eeb121686d281c98e5/ui/file_manager/integration_tests/file_manager/quick_view.js
,
Jun 14 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/58ed5a3222012df0952d1f5e67671eb45ea861fd commit 58ed5a3222012df0952d1f5e67671eb45ea861fd Author: Noel Gordon <noel@chromium.org> Date: Thu Jun 14 01:12:02 2018 Use descendant combinator in openQuickViewUsb browser test Code search for USB_VOLUME_QUERY suggests many uses of child selectors from the #directory-tree. Alternative is to use descendant combinator: #directory-tree [volume-type-icon="xxxxx"] to query for volumes, which is shorter and more idiomatic with the volume code selectors. Unless we need to test a child element for [active], or similar, it is perhaps less confusing to use the descendant combinator [1]. [1] Future changes might clean up the use of child selectors in volume MTP|USB|ETC_VOLUME_QUERY in the relevant test cases. No change in behavior, no new tests. Test: browser_test --gtest_filter="QuickView/FilesApp*" Bug: 851888 Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation Change-Id: I2f64c624e05d0051750ee49ed6e7fabbef43b512 Reviewed-on: https://chromium-review.googlesource.com/1098905 Reviewed-by: Naoki Fukino <fukino@chromium.org> Commit-Queue: Noel Gordon <noel@chromium.org> Cr-Commit-Position: refs/heads/master@{#567075} [modify] https://crrev.com/58ed5a3222012df0952d1f5e67671eb45ea861fd/ui/file_manager/integration_tests/file_manager/quick_view.js
,
Jun 14 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2b6aa94a27ca8ae0d38857b9bf8212bdebf7a2c1 commit 2b6aa94a27ca8ae0d38857b9bf8212bdebf7a2c1 Author: Noel Gordon <noel@chromium.org> Date: Thu Jun 14 01:13:56 2018 Use descendant combinator in copyBetweenWindows browser tests Use a decendant combinator selector to query for the USB volume: it is shorter and more idiomatic with other volume selector JS test code. Test: browser_test --gtest_filter="*FilesApp*copyBetweenWindows*" Bug: 836254, 851888 Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation Change-Id: I70b9ecad9e57db6a2d0028ba329707c5574710e8 Reviewed-on: https://chromium-review.googlesource.com/1098908 Reviewed-by: Naoki Fukino <fukino@chromium.org> Commit-Queue: Noel Gordon <noel@chromium.org> Cr-Commit-Position: refs/heads/master@{#567077} [modify] https://crrev.com/2b6aa94a27ca8ae0d38857b9bf8212bdebf7a2c1/ui/file_manager/integration_tests/file_manager/copy_between_windows.js
,
Jun 14 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ab508cbb92101f3db0144c0ba727afa35b551330 commit ab508cbb92101f3db0144c0ba727afa35b551330 Author: Noel Gordon <noel@chromium.org> Date: Thu Jun 14 01:14:10 2018 Use descendant combinator in FileDisplay USB,MTP browser tests Use a descendant combinator CSS selector to query for USB/MTP volumes: it is shorter and more idiomatic with volume code selectors. Other clean-up while here: add a comment for load File app step. Where an argument in a test step function is unused, remove it. Test: browser_test --gtest_filter="FileDisplay/FilesApp*" Bug: 836254, 851888 Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation Change-Id: I88b04907ed04b355d8de9412ba748ca1ce0ced21 Reviewed-on: https://chromium-review.googlesource.com/1098907 Reviewed-by: Naoki Fukino <fukino@chromium.org> Commit-Queue: Noel Gordon <noel@chromium.org> Cr-Commit-Position: refs/heads/master@{#567078} [modify] https://crrev.com/ab508cbb92101f3db0144c0ba727afa35b551330/ui/file_manager/integration_tests/file_manager/file_display.js
,
Jun 19 2018
,
Jun 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ab55380088b936ecffc895080227a36e3d6f5849 commit ab55380088b936ecffc895080227a36e3d6f5849 Author: Noel Gordon <noel@chromium.org> Date: Tue Jun 19 09:25:35 2018 Simplify setupAndWaitUntilReady in QuickView browser tests The QuickView tests are slower than most, and the bug addresses making them faster. setupAndWaitUntilReady loads, and pre-populates Downloads and Drive volumes with many file entries. Can we do better? A modest test speed improvement is to make setupAndWaitUntilReady only populate the volumes needed in the tests (Downloads, Drive, etc), with only those files needed to perform each test. Bug: 851888 Cq-Include-Trybots: luci.chromium.try:closure_compilation Change-Id: I0cb4d08fa38d9df76999595f41dfb8a0df378a9f Reviewed-on: https://chromium-review.googlesource.com/1104619 Reviewed-by: Sam McNally <sammc@chromium.org> Commit-Queue: Noel Gordon <noel@chromium.org> Cr-Commit-Position: refs/heads/master@{#568378} [modify] https://crrev.com/ab55380088b936ecffc895080227a36e3d6f5849/ui/file_manager/integration_tests/file_manager/quick_view.js
,
Jun 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8615321de3032322501be23dde2f71740342bb50 commit 8615321de3032322501be23dde2f71740342bb50 Author: Noel Gordon <noel@chromium.org> Date: Tue Jun 19 13:02:19 2018 Document the Quick View browser test helpers Now the Quick View browser test suite has been optimized for speed, we can finish off by documenting the Quick View test JS. Two test helpers only lack documentation: document what they do. Comment only change: no change in behavior, no new tests. Bug: 851888 Change-Id: If87c49919cdd3bd51fa35f714130fc74fb84b543 Reviewed-on: https://chromium-review.googlesource.com/1105530 Reviewed-by: Sam McNally <sammc@chromium.org> Commit-Queue: Noel Gordon <noel@chromium.org> Cr-Commit-Position: refs/heads/master@{#568419} [modify] https://crrev.com/8615321de3032322501be23dde2f71740342bb50/ui/file_manager/integration_tests/file_manager/quick_view.js
,
Jun 19 2018
At this point, the simplest possible file is used to open QuickView (#3 use a text file rather than a PNG), and minimize the number of files added to the test volumes during test (#10). This makes the current Quick View tests about as fast as possible. They are not doing anything silly, like playing 5 seconds of Audio on the bots. They're very basic tests of a Files.App feature (Quick View), and coverage has been expanded in this bug. The litmus test then is, what are the affects on MSAN/ASAN bots with all these changes? Have things improved? In a few days, we'll have that data...
,
Jun 29 2018
The recent speed-up in the MSAN bots was noted in related FilesApp browser test bugs. I'm yet to identify the change that improved it. But the first effect was test-time reduced by about 30%. Instead of the FilesApp QuickView tests persistently hitting the MSAN bot time-out (180s) with a TIMEOUT PASS, TIMEOUT PASS ... flake, they improved to have average test-time 138s, with no flakes at all. More work must have happened on the MSAN bot configs as of today ... the last 4 runs of the FilesApp QuickView tests on MSAN now average 40s. We have gone from 180s test-run-time and TIMEOUT PASS flakes, to 40s test-run-time and no flakes as of today [1]. https://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=QuickView%2FFilesAppBrowserTest ^^^ result looks very good to me: green everywhere RELEASE/DEBUG/MSAN/ASAN and no flakes.
,
Jun 29 2018
Nice work Noel.
,
Jun 30 2018
Thanks Ken. I was wondering if you or Dirk knew of any recent infra work on the chrome-os MSAN bots? Those bots became way faster in the last week or so. Someone else in the team has found a massive MSAN perf win by fixing a bug or reverting something. I noticed all the FilesApp browser test suite (that's 270 fixtures) became faster on MSAN this week. The tests in this bug (180/40 - 1) * 100% = 350% faster.
,
Jun 30 2018
Sorry, I don't know anything about those bots.
,
Jul 3
No worries. Snapshot of the flakes going away and the speed increases.
,
Jul 3
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by noel@chromium.org
, Jun 12 2018Status: Started (was: Available)