Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(11)

Issue 14851005: SyncFS: Fix LocalSyncOperationResolverTest to cover all test cases (Closed)

Created:
7 years, 7 months ago by nhiroki
Modified:
7 years, 7 months ago
Reviewers:
kinuko
CC:
chromium-reviews, tzik+watch_chromium.org, kinuko+watch, tzik, calvinlo
Visibility:
Public.

Description

SyncFS: Fix LocalSyncOperationResolverTest to cover all test cases To cover all test cases, this CL removes hard-coded parameters and adds inputs. BUG=none TEST=unit_tests --gtest_filter=\*LocalSyncOperationResolver\ Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=198773

Patch Set 1 : #

Total comments: 4

Patch Set 2 : review fix #

Total comments: 8

Patch Set 3 : review fix #

Total comments: 4

Patch Set 4 : review fix #

Unified diffs Side-by-side diffs Delta from patch set Stats (+161 lines, -152 lines) Patch
M chrome/browser/sync_file_system/drive_file_sync_service.cc View 1 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/sync_file_system/local_sync_operation_resolver.h View 1 2 1 chunk +12 lines, -20 lines 0 comments Download
M chrome/browser/sync_file_system/local_sync_operation_resolver.cc View 1 2 3 14 chunks +38 lines, -56 lines 0 comments Download
M chrome/browser/sync_file_system/local_sync_operation_resolver_unittest.cc View 1 9 chunks +110 lines, -74 lines 0 comments Download

Messages

Total messages: 13 (0 generated)
nhiroki
PTAL, thanks!
7 years, 7 months ago (2013-05-02 12:46:51 UTC) #1
kinuko
https://codereview.chromium.org/14851005/diff/4001/chrome/browser/sync_file_system/local_sync_operation_resolver_unittest.cc File chrome/browser/sync_file_system/local_sync_operation_resolver_unittest.cc (right): https://codereview.chromium.org/14851005/diff/4001/chrome/browser/sync_file_system/local_sync_operation_resolver_unittest.cc#newcode19 chrome/browser/sync_file_system/local_sync_operation_resolver_unittest.cc:19: FileChange remote_file_change; Maybe this could be scoped_ptr<FileChange> remote_file_change and ...
7 years, 7 months ago (2013-05-07 04:26:10 UTC) #2
nhiroki
Thanks, updated. https://codereview.chromium.org/14851005/diff/4001/chrome/browser/sync_file_system/local_sync_operation_resolver_unittest.cc File chrome/browser/sync_file_system/local_sync_operation_resolver_unittest.cc (right): https://codereview.chromium.org/14851005/diff/4001/chrome/browser/sync_file_system/local_sync_operation_resolver_unittest.cc#newcode19 chrome/browser/sync_file_system/local_sync_operation_resolver_unittest.cc:19: FileChange remote_file_change; On 2013/05/07 04:26:10, kinuko wrote: ...
7 years, 7 months ago (2013-05-07 11:13:17 UTC) #3
kinuko
looks good, some more nits https://codereview.chromium.org/14851005/diff/18001/chrome/browser/sync_file_system/local_sync_operation_resolver.h File chrome/browser/sync_file_system/local_sync_operation_resolver.h (right): https://codereview.chromium.org/14851005/diff/18001/chrome/browser/sync_file_system/local_sync_operation_resolver.h#newcode33 chrome/browser/sync_file_system/local_sync_operation_resolver.h:33: static LocalSyncOperationType Resolve( Can ...
7 years, 7 months ago (2013-05-07 11:20:16 UTC) #4
kinuko
https://codereview.chromium.org/14851005/diff/18001/chrome/browser/sync_file_system/local_sync_operation_resolver_unittest.cc File chrome/browser/sync_file_system/local_sync_operation_resolver_unittest.cc (right): https://codereview.chromium.org/14851005/diff/18001/chrome/browser/sync_file_system/local_sync_operation_resolver_unittest.cc#newcode72 chrome/browser/sync_file_system/local_sync_operation_resolver_unittest.cc:72: return vector.Pass(); On 2013/05/07 11:20:16, kinuko wrote: > Is ...
7 years, 7 months ago (2013-05-07 11:25:25 UTC) #5
nhiroki
PTAL https://codereview.chromium.org/14851005/diff/18001/chrome/browser/sync_file_system/local_sync_operation_resolver.h File chrome/browser/sync_file_system/local_sync_operation_resolver.h (right): https://codereview.chromium.org/14851005/diff/18001/chrome/browser/sync_file_system/local_sync_operation_resolver.h#newcode33 chrome/browser/sync_file_system/local_sync_operation_resolver.h:33: static LocalSyncOperationType Resolve( On 2013/05/07 11:20:16, kinuko wrote: ...
7 years, 7 months ago (2013-05-07 12:20:25 UTC) #6
kinuko
lgtm
7 years, 7 months ago (2013-05-07 12:29:54 UTC) #7
kinuko
https://codereview.chromium.org/14851005/diff/21001/chrome/browser/sync_file_system/local_sync_operation_resolver.cc File chrome/browser/sync_file_system/local_sync_operation_resolver.cc (right): https://codereview.chromium.org/14851005/diff/21001/chrome/browser/sync_file_system/local_sync_operation_resolver.cc#newcode17 chrome/browser/sync_file_system/local_sync_operation_resolver.cc:17: if (remote_file_change != NULL && remote_file_change->IsTypeUnknown()) nit: maybe just ...
7 years, 7 months ago (2013-05-07 12:30:05 UTC) #8
nhiroki
Thanks! https://codereview.chromium.org/14851005/diff/21001/chrome/browser/sync_file_system/local_sync_operation_resolver.cc File chrome/browser/sync_file_system/local_sync_operation_resolver.cc (right): https://codereview.chromium.org/14851005/diff/21001/chrome/browser/sync_file_system/local_sync_operation_resolver.cc#newcode17 chrome/browser/sync_file_system/local_sync_operation_resolver.cc:17: if (remote_file_change != NULL && remote_file_change->IsTypeUnknown()) On 2013/05/07 ...
7 years, 7 months ago (2013-05-07 12:41:33 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/nhiroki@chromium.org/14851005/18002
7 years, 7 months ago (2013-05-07 12:41:49 UTC) #10
commit-bot: I haz the power
Sorry for I got bad news for ya. Compile failed with a clobber build on ...
7 years, 7 months ago (2013-05-07 12:48:56 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/nhiroki@chromium.org/14851005/18002
7 years, 7 months ago (2013-05-07 13:13:33 UTC) #12
commit-bot: I haz the power
7 years, 7 months ago (2013-05-07 18:57:03 UTC) #13
Message was sent while issue was closed.
Change committed as 198773

Powered by Google App Engine
This is Rietveld 408576698