New issue
Advanced search Search tips

Issue 692034 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Wrong log message in some test cases in browser_tests

Project Member Reported by yamaguchi@chromium.org, Feb 14 2017

Issue description

Chrome Version: ToT

The log output from this line becomes unexpected in some tests.
https://cs.chromium.org/chromium/src/extensions/renderer/resources/test_custom_bindings.js?type=cs&l=120


REPRO STEPS:
browser_tests --gtest_filter=GalleryBrowserTest.RotateImageOnDrive

EXPECTED:
...:INFO:CONSOLE(0)] "[SUCCESS] rotateImageOnDrive", source: chrome-extension://ejhcmmdhhpdhhgmifplfmjobgegbibkn/_generated_background_page.html (0)

ACTUAL:
...:INFO:CONSOLE(0)] "[SUCCESS] undefined", source: chrome-extension://ejhcmmdhhpdhhgmifplfmjobgegbibkn/_generated_background_page.html (0)

The actual contents of the variable (function) referred in the line is wrapping the actual target test, thus has no .name or .generatedName field.
> "function () {
>        return testPromiseAndApps(targetTest(), [gallery]);
>      }"

 

Comment 1 by sashab@chromium.org, Feb 24 2018

Labels: CrOS-FilesApp-CodeHealth

Comment 2 by sashab@chromium.org, Feb 28 2018

Labels: -CrOS-FilesApp-CodeHealth CrOSFilesCategory-CodeHealth

Comment 3 by sashab@chromium.org, Feb 28 2018

Labels: -CrOSFilesCategory-CodeHealth CrOSFilesCategory-Testing

Comment 4 by sashab@chromium.org, Apr 12 2018

Cc: noel@chromium.org
Status: Available (was: Untriaged)

Comment 5 by noel@chromium.org, Apr 19 2018

yamaguchi-san@ I have certainly seen the "[SUCCESS] undefined" output when dealing with our integration tests.  Mentioned this to you today in passing.

Comment 6 by noel@chromium.org, Apr 20 2018

Cc: -noel@chromium.org yamaguchi@chromium.org
Owner: noel@chromium.org
Status: Started (was: Available)
I have a fix for this.
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 21 2018

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

commit 447e96ae9f60d4e716d97385d2f4480aed2fc498
Author: Noel Gordon <noel@chromium.org>
Date: Sat Apr 21 04:44:39 2018

Tests in the testcase namespace should be anon Function

Two files in the testcase namespace do not use anon Function while all
other files do. We are all individuals, but there's little sense being
different here, it buys us nothing [1].

Fix those files (providers.js, transfer.js) to use anon Functions. Add
comments to each test. Add TODO about improving test names.

No change in behavior, no new tests.

[1] An upcoming CL will enforce that testcase namespace tests are anon
Function to make our preference for testcase code consistency clear.

Tbr: yamaguchi-san
Bug:  692034 ,  833834 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I955c64a24b4b063f9b8eb02a3a12b3a8816ad7b3
Reviewed-on: https://chromium-review.googlesource.com/1023350
Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552575}
[modify] https://crrev.com/447e96ae9f60d4e716d97385d2f4480aed2fc498/ui/file_manager/integration_tests/file_manager/providers.js
[modify] https://crrev.com/447e96ae9f60d4e716d97385d2f4480aed2fc498/ui/file_manager/integration_tests/file_manager/transfer.js

Project Member

Comment 8 by bugdroid1@chromium.org, Apr 21 2018

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

commit 3c18a85d5515e527707def269fd027b59bdef464
Author: Noel Gordon <noel@chromium.org>
Date: Sat Apr 21 09:21:52 2018

FileManagerBrowserTest: log test case name

Rather than LOG the test extension name, LOG the test case name.

Tbr: yamaguchi-san
Bug:  692034 ,  833834 
Change-Id: I6b2cf99860fa937d4801a5d7236151d7bb013f44
Reviewed-on: https://chromium-review.googlesource.com/1023490
Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552586}
[modify] https://crrev.com/3c18a85d5515e527707def269fd027b59bdef464/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Apr 23 2018

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

commit 14bb2018097509c5f3f615af2a550fced4ad3789
Author: Noel Gordon <noel@chromium.org>
Date: Mon Apr 23 02:46:06 2018

Test extensions "[Success] undefined" messages are unhelpful

The message is printed on test success in the test extension JS, since
testcase namespace test functions are anon JS Function and chrome.test
prints their Function.name property as "undefined" it seems.

  "No function in a framework trace should have a useless name. And
   the most useless name of all is: (anonymous function)."

Better to show the actual test name in the "[SUCCESS] ..." message but
a Function .name property is not writable. Is there a work-around?

Change the test extension background.js test drivers: add comments for
each step. In the step that runs the test, assert the test function is
Function type with no name (anon) [1].

Next, duct tape the desired test case name onto the test function: via
a Symbol property of an Object with a property value wrapping the test
function. Thus, Object[Symbol] is a Function with .name property equal
to the desired test case name, and a body equal to the test function.

Object[Symbol] can then be used to call it via chrome.test.RunTests so
do that. And by these semi-magical incantations, the desired test case
name (defined by the Symbol) appears: "SUCCESS test case name".

[1] See also crrev.com/552575

Bug:  692034 ,  833834 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ie3e8846df053d90722e7d7d15ddd120d758a2224
Reviewed-on: https://chromium-review.googlesource.com/1021100
Reviewed-by: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552623}
[modify] https://crrev.com/14bb2018097509c5f3f615af2a550fced4ad3789/ui/file_manager/integration_tests/audio_player/background.js
[modify] https://crrev.com/14bb2018097509c5f3f615af2a550fced4ad3789/ui/file_manager/integration_tests/file_manager/background.js
[modify] https://crrev.com/14bb2018097509c5f3f615af2a550fced4ad3789/ui/file_manager/integration_tests/gallery/background.js
[modify] https://crrev.com/14bb2018097509c5f3f615af2a550fced4ad3789/ui/file_manager/integration_tests/video_player/background.js

Comment 10 by noel@chromium.org, Apr 23 2018

Status: Fixed (was: Started)

Sign in to add a comment