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

Issue 671283 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

sessions_sync_manager_unittest should be componentized

Project Member Reported by zea@chromium.org, Dec 5 2016

Issue description

Now that we have componentized Sessions, and the ChromeSyncClient (along with the various getters for the window/tab info) exist, the SessionSyncManager unit tests should become proper unit tests and stop depending on BrowserWithTestWindowTest.

Currently they rely on AddTab/NavigateAndCommitActiveTab as helpers to avoid setting up tabs properly. We should just build those helpers ourselves and rely on the actual integration tests (in sync integration tests) to verify the dependency between the browser and sessions logic.
 

Comment 1 by s...@chromium.org, Dec 5 2016

Cc: s...@chromium.org

Comment 2 by s...@chromium.org, Dec 8 2016

 Issue 544973  has been merged into this issue.
Summary: sessions_sync_manager_unittest should be componentized (was: sessions_sync_manager_unittest should stop depending on BrowserWithTestWindowTest)
Updating summary to make explicit that the test should end up in components_unittests.
Project Member

Comment 4 by bugdroid1@chromium.org, Feb 23 2017

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

commit 5bdcd6fff79307e080a87ee417960f2e2141dc24
Author: zea <zea@chromium.org>
Date: Thu Feb 23 00:58:49 2017

[Sync] Refactor SessionsSyncManager unit tests

Tests no longer rely on BrowserWithTestTestWindowTest, and instead inject
all the relevant navigation events directly. This allows for much more control
over what gets tested, and in particular allows for testing situations with
multiple windows.

As part of this cleanup, various helper methods were added to make the tests
simpler and more readable. A couple outdated tests were also removed.

Lastly, because there are no longer any content dependencies, the test was
moved to components/sync_sessions, to live next to the code it's actually
testing (SessionsSyncManager).

BUG= 639009 ,  671283 

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

[delete] https://crrev.com/169beb0367bc0ca03c4cb0baa18d183740f157d0/chrome/browser/sync/sessions/sessions_sync_manager_unittest.cc
[modify] https://crrev.com/5bdcd6fff79307e080a87ee417960f2e2141dc24/chrome/test/BUILD.gn
[modify] https://crrev.com/5bdcd6fff79307e080a87ee417960f2e2141dc24/components/sync_sessions/BUILD.gn
[add] https://crrev.com/5bdcd6fff79307e080a87ee417960f2e2141dc24/components/sync_sessions/sessions_sync_manager_unittest.cc

Comment 5 by zea@chromium.org, Feb 23 2017

Status: Fixed (was: Assigned)
Hurray!
\o/

Sign in to add a comment