New issue
Advanced search Search tips

Issue 908435 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 17
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug



Sign in to add a comment

Enabling "OmniboxDocumentProvider" breaks browser test

Project Member Reported by krb@google.com, Nov 26

Issue description

Chrome Version: ToT (72.0.3616.0)
OS: Linux

What steps will reproduce the problem?
(1) Enable "OmniboxDocumentProvider" in config tests

+++ b/testing/variations/fieldtrial_testing_config.json
@@ -3270,6 +3270,7 @@
                     },
                     "enable_features": [
                         "OmniboxDisplayTitleForCurrentUrl",
+                        "OmniboxDocumentProvider",
                         "OmniboxNewAnswerLayout",

(2) On Linux e.g., build and run:

browser_tests --gtest_filter=CrSettingsPeoplePageSyncPageTest.All

What is the expected result?

Test passes.

What happens instead?
Test fails. Error below.

It would be nice if we could enable this feature during the test (or a test), so that the warning from Finch about the untested feature could go away. (It would also be good to figure out why it would change behavior, which may be perfectly benign.)

[57772:57772:1126/064125.002251:ERROR:web_ui_test_handler.cc(100)] Test Errors: 1/27 tests had failed assertions.
[57772:57772:1126/064125.002513:ERROR:web_ui_browser_test.cc(493)] CONDITION FAILURE: encountered javascript console error(s):
[57772:57772:1126/064125.002570:ERROR:web_ui_browser_test.cc(495)] JS ERROR: '[57772:57772:1126/064116.052937:ERROR:CONSOLE(48)] "Mocha test failed: AdvancedSyncSettingsTests SyncSectionLayout_UnifiedConsentEnabled_SignedIn
AssertionError: expected 3 to equal 4
    at Function.assert.strictEqual (file:///usr/local/google/home/krb/Chrome2/src/third_party/chaijs/chai.js:2277:32)
    at assertEquals (file:///usr/local/google/home/krb/Chrome2/src/chrome/test/data/webui/test_api.js:923:15)
    at Context.<anonymous> (file:///usr/local/google/home/krb/Chrome2/src/chrome/test/data/webui/settings/people_page_sync_page_test.js:223:7)
", source: file:///usr/local/google/home/krb/Chrome2/src/chrome/test/data/webui/mocha_adapter.js (48)
'
[57772:57772:1126/064125.002654:ERROR:web_ui_browser_test.cc(497)] JS call assumed failed, because JS console error(s) found.
gen/chrome/test/data/webui/settings/cr_settings_browsertest-gen.cc:846: Failure
Value of: RunJavascriptTestF( true, "CrSettingsPeoplePageSyncPageTest", "All")
  Actual: false
Expected: true

 
Cc: -skare@chromium.org
Components: UI>Browser>Omnibox
Owner: skare@chromium.org
Status: Assigned (was: Available)
Status: Started (was: Assigned)
The settings webui changes when the feature is on, to add a toggle for it, so it can be 3 or 4. 

Should be an easy fix, but looking at what other features have done in such cases. 
This is now being enabled by default in field_trial_config, so changing the test. Using this bug for reference.

The code can be removed when the feature is on by default, which might take a couple milestones + code traveling to stable.
Project Member

Comment 4 by bugdroid1@chromium.org, Jan 16

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

commit ac1ed69b3b6b74b0531fb6886ac418b1b3f2267c
Author: Travis Skare <skare@chromium.org>
Date: Wed Jan 16 02:03:39 2019

Add OmniboxDocumentProver to field trial testing config,

Update tests that enabling the experiment would otherwise break.

BUG:  908435 

Change-Id: I63eb6444c576172beae9a6cbdefddc03a812b6fd
Reviewed-on: https://chromium-review.googlesource.com/c/1407746
Reviewed-by: Hector Carmona <hcarmona@chromium.org>
Reviewed-by: Rebekah Potter <rbpotter@chromium.org>
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Commit-Queue: Travis Skare <skare@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623024}
[modify] https://crrev.com/ac1ed69b3b6b74b0531fb6886ac418b1b3f2267c/chrome/test/data/webui/settings/cr_settings_browsertest.js
[modify] https://crrev.com/ac1ed69b3b6b74b0531fb6886ac418b1b3f2267c/chrome/test/data/webui/settings/people_page_sync_page_test.js
[modify] https://crrev.com/ac1ed69b3b6b74b0531fb6886ac418b1b3f2267c/testing/variations/fieldtrial_testing_config.json

Comment 5 by skare@chromium.org, Jan 17 (5 days ago)

Status: Fixed (was: Started)

Sign in to add a comment