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

Issue 2444313002: Remove function casting from Libsecret tests (Closed)

Created:
4 years, 1 month ago by cfroussios
Modified:
4 years, 1 month ago
Reviewers:
vasilii
CC:
chromium-reviews
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Remove function casting from Libsecret tests Some libsecret data types are hidden. For our tests, we introduce our own data types and cast them to the expected types. Previously, that was done by casting the mocked functions. This is undefined behaviour, thefore we are moving the casting inside the mock methods. BUG=658723 Committed: https://crrev.com/8a42817875f7395f54bc567a319b79062b489261 Cr-Commit-Position: refs/heads/master@{#427331}

Patch Set 1 #

Patch Set 2 : fix for password manager too #

Total comments: 6

Patch Set 3 : feedback #

Patch Set 4 : better message #

Patch Set 5 : typo #

Unified diffs Side-by-side diffs Delta from patch set Stats (+45 lines, -40 lines) Patch
M chrome/browser/password_manager/native_backend_libsecret_unittest.cc View 1 2 chunks +15 lines, -18 lines 0 comments Download
M components/os_crypt/key_storage_libsecret_unittest.cc View 1 2 3 4 6 chunks +16 lines, -14 lines 0 comments Download
M components/os_crypt/libsecret_util_linux.cc View 1 chunk +14 lines, -8 lines 0 comments Download

Messages

Total messages: 25 (18 generated)
cfroussios
4 years, 1 month ago (2016-10-25 09:34:22 UTC) #4
vasilii
https://codereview.chromium.org/2444313002/diff/20001/components/os_crypt/key_storage_libsecret_unittest.cc File components/os_crypt/key_storage_libsecret_unittest.cc (right): https://codereview.chromium.org/2444313002/diff/20001/components/os_crypt/key_storage_libsecret_unittest.cc#newcode16 components/os_crypt/key_storage_libsecret_unittest.cc:16: // cast to the correct signature. We can reduce ...
4 years, 1 month ago (2016-10-25 11:42:57 UTC) #7
cfroussios
https://codereview.chromium.org/2444313002/diff/20001/components/os_crypt/key_storage_libsecret_unittest.cc File components/os_crypt/key_storage_libsecret_unittest.cc (right): https://codereview.chromium.org/2444313002/diff/20001/components/os_crypt/key_storage_libsecret_unittest.cc#newcode16 components/os_crypt/key_storage_libsecret_unittest.cc:16: // cast to the correct signature. We can reduce ...
4 years, 1 month ago (2016-10-25 12:14:40 UTC) #12
vasilii
lgtm
4 years, 1 month ago (2016-10-25 12:19:57 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2444313002/80001
4 years, 1 month ago (2016-10-25 13:11:58 UTC) #22
commit-bot: I haz the power
Committed patchset #5 (id:80001)
4 years, 1 month ago (2016-10-25 13:16:41 UTC) #23
commit-bot: I haz the power
4 years, 1 month ago (2016-10-25 13:20:13 UTC) #25
Message was sent while issue was closed.
Patchset 5 (id:??) landed as
https://crrev.com/8a42817875f7395f54bc567a319b79062b489261
Cr-Commit-Position: refs/heads/master@{#427331}

Powered by Google App Engine
This is Rietveld 408576698