New issue
Advanced search Search tips

Issue 891148 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 10
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug

Blocking:
issue 836254



Sign in to add a comment

sendTestMessage returns undefined sometimes

Project Member Reported by noel@chromium.org, Oct 2

Issue description

Running the QuickView*FilesApp browser tests locally in parallel in DEBUG mode, for example

  xvfb-run -s "-screen 0 1024x1024x24" \
    ./out/DEBUG/browser_tests --gtest_filter="QuickView*" \
       --gtest_repeat=100 --test-launcher-jobs=24

can sometimes lead to a TIME_OUT failure, about 1 time in 100 repeats. 

The failure happens in the getTestName command sendTestMessage call in the FilesApp test extension.

The expected return is a string (the test case name), but is undefined sometimes, causing a failure at:

https://cs.chromium.org/chromium/src/ui/file_manager/integration_tests/file_manager/background.js?dr&g=0&l=464

reading the test name: "cannot access property name of undefined".

We should make the command requester more robust: it should check for a string return, and retry the command if not.
 
Status: Started (was: Untriaged)
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 2

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

commit 86c8d52604436bce32f08db7defee5297112f90a
Author: Noel Gordon <noel@chromium.org>
Date: Tue Oct 02 05:45:23 2018

Integration tests: add sendBrowserTestCommand helper

Add sendBrowserTestCommand that expects a 'string' result: this helper
retries until a 'string' result is received, and logs errors.

Update sendTestMessage docs (change message -> command) and update the
test extension harness to use sendBrowserTestCommand.

Bug:  891148 
Change-Id: I5840668919672e68787eccd8dece3eae5f2d8c45
Reviewed-on: https://chromium-review.googlesource.com/1256482
Reviewed-by: Joel Hockey <joelhockey@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595733}
[modify] https://crrev.com/86c8d52604436bce32f08db7defee5297112f90a/ui/file_manager/integration_tests/file_manager/background.js
[modify] https://crrev.com/86c8d52604436bce32f08db7defee5297112f90a/ui/file_manager/integration_tests/test_util.js

Project Member

Comment 3 by bugdroid1@chromium.org, Oct 9

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

commit e1f777a4f2ba4bdf16e04739de18730e6d50ecc6
Author: Noel Gordon <noel@chromium.org>
Date: Tue Oct 09 06:29:27 2018

Integration tests: use sendBrowserTestCommand helper everywhere

CL:1256482 made FilesAppBrowserTest use sendBrowserTestCommand to send
JS test command messages to the browser test harness.

Use it everywhere (video player, audio player, and gallery tests) as a
preventative to avoid the issues mentioned on the bug.

Bug:  891148 
Change-Id: Ie900ba11065cf4fc6b4537ad34be19a1f8d79ab4
Reviewed-on: https://chromium-review.googlesource.com/c/1270595
Reviewed-by: Trent Apted <tapted@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597817}
[modify] https://crrev.com/e1f777a4f2ba4bdf16e04739de18730e6d50ecc6/ui/file_manager/integration_tests/audio_player/background.js
[modify] https://crrev.com/e1f777a4f2ba4bdf16e04739de18730e6d50ecc6/ui/file_manager/integration_tests/gallery/background.js
[modify] https://crrev.com/e1f777a4f2ba4bdf16e04739de18730e6d50ecc6/ui/file_manager/integration_tests/video_player/background.js

Project Member

Comment 4 by bugdroid1@chromium.org, Oct 10

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

commit a8ea3994c3cffa22a3cd8a07cf51eb5e6ce504bd
Author: Noel Gordon <noel@chromium.org>
Date: Wed Oct 10 02:11:44 2018

Integration tests: use sendTestMessage helper everywhere

Don't call chrome.test.sendMessage() directly to send a command to the
browser test harness. Use the helper routine instead.

Bug:  891148 
Change-Id: I44974abeae92f789827477003350b88494b4f585
Reviewed-on: https://chromium-review.googlesource.com/c/1272336
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598179}
[modify] https://crrev.com/a8ea3994c3cffa22a3cd8a07cf51eb5e6ce504bd/ui/file_manager/integration_tests/file_manager/copy_between_windows.js
[modify] https://crrev.com/a8ea3994c3cffa22a3cd8a07cf51eb5e6ce504bd/ui/file_manager/integration_tests/file_manager/drive_specific.js
[modify] https://crrev.com/a8ea3994c3cffa22a3cd8a07cf51eb5e6ce504bd/ui/file_manager/integration_tests/file_manager/file_display.js
[modify] https://crrev.com/a8ea3994c3cffa22a3cd8a07cf51eb5e6ce504bd/ui/file_manager/integration_tests/file_manager/my_files.js
[modify] https://crrev.com/a8ea3994c3cffa22a3cd8a07cf51eb5e6ce504bd/ui/file_manager/integration_tests/file_manager/providers.js
[modify] https://crrev.com/a8ea3994c3cffa22a3cd8a07cf51eb5e6ce504bd/ui/file_manager/integration_tests/file_manager/quick_view.js
[modify] https://crrev.com/a8ea3994c3cffa22a3cd8a07cf51eb5e6ce504bd/ui/file_manager/integration_tests/file_manager/suggest_app_dialog.js
[modify] https://crrev.com/a8ea3994c3cffa22a3cd8a07cf51eb5e6ce504bd/ui/file_manager/integration_tests/file_manager/zip_files.js

Blocking: 836254
Status: Fixed (was: Started)

Sign in to add a comment