New issue
Advanced search Search tips

Issue 689302 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Feb 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

MD Settings: CrSettingsDateTimePageTest.DateTimePageTest accidentally passing?

Project Member Reported by dpa...@chromium.org, Feb 7 2017

Issue description

Inside date_time_page.html there is a cr-pref-policy-indicator that refers accidentally to a non existing Polymer property, see [1] , where timeZonePolicyPref_ should have been fakeTimeZonePolicyPref_.

The corresponding tests seem to pass either way, which is makes me suspect that they are accidentally passing. The assertion at [2] seems a bit fragile too (perhaps there should be a more explicit way to ask the cr-policy-pref-indicator if it is active). On top of that, those tests fail when run in Vulcanized mode, see test logs at [3]. 


[1] https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/date_time_page/date_time_page.html?l=33
[2] https://cs.chromium.org/chromium/src/chrome/test/data/webui/settings/date_time_page_tests.js?l=124
[3] https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_ng/builds/358247/steps/browser_tests%20%28with%20patch%29/logs/CrSettingsDateTimePageTest.DateTimePageTest
 
Labels: OS-Chrome
FYI, this is potentially blocking  issue 673825 .
Owner: dpa...@chromium.org
Status: Assigned (was: Untriaged)
I've verified that this is indeed a bug that is not being caught by the corresponding tests. Will follow investigation with understanding why the current tests fail to catch the bug.
Status: Started (was: Assigned)
I have a candidate fix. Will follow up tomorrow.
Project Member

Comment 5 by bugdroid1@chromium.org, Feb 10 2017

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

commit 74b7a0510c3aca53680a8d65fdc65d384c1d04b6
Author: dpapad <dpapad@chromium.org>
Date: Fri Feb 10 02:29:24 2017

MD Settings: Fix timezone autodetect policy indicator.

The cr-policy-pref-indicator was referring to a non-existing property, but the
corresponding tests did not catch the bug, because they were asserting on a
condition that was not sufficient. Moreover, the tests did not fire a
'time-zone-auto-detect-policy' WebUI event which is necessary to initialize the
fakeTimeZonePolicyPref_ property, which controls the pref indicator.

BUG= 689302 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2683613003
Cr-Commit-Position: refs/heads/master@{#449524}

[modify] https://crrev.com/74b7a0510c3aca53680a8d65fdc65d384c1d04b6/chrome/browser/resources/settings/date_time_page/date_time_page.html
[modify] https://crrev.com/74b7a0510c3aca53680a8d65fdc65d384c1d04b6/chrome/test/data/webui/settings/date_time_page_tests.js
[modify] https://crrev.com/74b7a0510c3aca53680a8d65fdc65d384c1d04b6/ui/webui/resources/cr_elements/policy/cr_policy_pref_indicator.html
[modify] https://crrev.com/74b7a0510c3aca53680a8d65fdc65d384c1d04b6/ui/webui/resources/cr_elements/policy/cr_policy_pref_indicator.js

Comment 6 by dpa...@chromium.org, Feb 13 2017

Status: Fixed (was: Started)
Status: Verified (was: Fixed)

Sign in to add a comment