New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 771433 link

Starred by 1 user
Status: Fixed
Owner:
█ warx@chromium.org
Last visit > 30 days ago
Closed: Nov 2017
Cc:
derat@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Create PowerButtonTestBase

Project Member Reported by warx@chromium.org, Oct 4 2017 
Create PowerButtonTestBase for three reasons:

(1) we don't have power_button_controller_unittest.cc, related test coverage are in lock_state_controller_unittest.cc and tablet_power_button_controller_unittest.cc
(2) we have more power button screenshot tests coming, from  issue 712072 
(3) reduce code lines: there are many common setup code and common util methods

I think this cleanup makes sense. Ideally I like the way each controller has its individual unit test file.

Also, I would like to move power_button_controller from ash/wm to ash/system/power/, for one reason:
(1) not like lock_state_controller, it observes powerd event, so it is more close to ash/system/power/ folder.

derat@, what do you think?

Comment 1 by derat@chromium.org, Oct 4 2017 

Sure, sharing common code makes sense and I was also considering moving PowerButtonController. :-) Thanks!
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 6 2017 

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

commit 03a1fd2f524671201708da4a0e1192d76f795792
Author: Qiang Xu <warx@chromium.org>
Date: Fri Oct 06 23:21:12 2017

cros: create PowerButtonTestBase

Changes:
Extract common code to PowerButtonTestBase, to be prepared for
 crbug.com/712072 .
This CL is extracted from
https://chromium-review.googlesource.com/c/chromium/src/+/704193.

Bug:  771433 
Test: covered by tests
Change-Id: Idf34779a9a097f3916f0f235e45fcf8903fe25c2
Reviewed-on: https://chromium-review.googlesource.com/704104
Commit-Queue: Qiang(Joe) Xu <warx@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507219}
[modify] https://crrev.com/03a1fd2f524671201708da4a0e1192d76f795792/ash/BUILD.gn
[add] https://crrev.com/03a1fd2f524671201708da4a0e1192d76f795792/ash/system/power/power_button_test_base.cc
[add] https://crrev.com/03a1fd2f524671201708da4a0e1192d76f795792/ash/system/power/power_button_test_base.h
[modify] https://crrev.com/03a1fd2f524671201708da4a0e1192d76f795792/ash/system/power/tablet_power_button_controller_unittest.cc
[modify] https://crrev.com/03a1fd2f524671201708da4a0e1192d76f795792/ash/wm/lock_state_controller_unittest.cc
[modify] https://crrev.com/03a1fd2f524671201708da4a0e1192d76f795792/ash/wm/power_button_controller.cc
[modify] https://crrev.com/03a1fd2f524671201708da4a0e1192d76f795792/ash/wm/power_button_controller.h
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 12 2017 

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

commit 0a86020fd2c566ae4d91161c4bcf304968bc8ab1
Author: Qiang Xu <warx@chromium.org>
Date: Thu Oct 12 08:34:54 2017

cros: move PowerButtonController to ash/system/power/

changes:
mechanical movement done by tools/git/move_source_file.py

Bug:  771433 
Test: compiles & ash_unittests
Change-Id: I55d1332292e7c0c921c9436c5fe464f1aa0f8c5d
Reviewed-on: https://chromium-review.googlesource.com/714697
Reviewed-by: Dan Erat <derat@chromium.org>
Commit-Queue: Qiang(Joe) Xu <warx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508284}
[modify] https://crrev.com/0a86020fd2c566ae4d91161c4bcf304968bc8ab1/ash/BUILD.gn
[modify] https://crrev.com/0a86020fd2c566ae4d91161c4bcf304968bc8ab1/ash/accelerators/accelerator_controller_delegate_classic.cc
[modify] https://crrev.com/0a86020fd2c566ae4d91161c4bcf304968bc8ab1/ash/shell.cc
[modify] https://crrev.com/0a86020fd2c566ae4d91161c4bcf304968bc8ab1/ash/shell_test_api.cc
[rename] https://crrev.com/0a86020fd2c566ae4d91161c4bcf304968bc8ab1/ash/system/power/power_button_controller.cc
[rename] https://crrev.com/0a86020fd2c566ae4d91161c4bcf304968bc8ab1/ash/system/power/power_button_controller.h
[modify] https://crrev.com/0a86020fd2c566ae4d91161c4bcf304968bc8ab1/ash/system/power/power_button_test_base.cc
[modify] https://crrev.com/0a86020fd2c566ae4d91161c4bcf304968bc8ab1/ash/system/power/power_button_test_base.h
[modify] https://crrev.com/0a86020fd2c566ae4d91161c4bcf304968bc8ab1/ash/system/power/tablet_power_button_controller_unittest.cc
[modify] https://crrev.com/0a86020fd2c566ae4d91161c4bcf304968bc8ab1/ash/wm/lock_state_controller_unittest.cc

Comment 4 by warx@chromium.org, Nov 8 2017

Status: Fixed (was: Assigned)
This is mostly done. I will evaluate whether it makes sense to create a power_button_controller_unittest.cc file to host some test cases from lock_state_controller_unittest.cc file.

Comment 5 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Comment 6 by dchan@chromium.org, Jan 23 2018

Status: Fixed (was: Archived)
►

Sign in to add a comment