zios-9245 remove keychain testing logic which prevent deleting items #72
Conversation
billypchan
changed the title
| @@ -0,0 +1,23 @@ | |||
| //// | |||
| // Wire | |||
| // Copyright (C) 2017 Wire Swiss GmbH | |||
There was a problem hiding this comment.
| Your copyright header is so last year! |
There was a problem hiding this comment.
This file was created last year!
Generated by 🚫 Danger |
Codecov Report
@@ Coverage Diff @@
## develop #72 +/- ##
===========================================
+ Coverage 74.81% 75.68% +0.87%
===========================================
Files 42 42
Lines 3724 3722 -2
===========================================
+ Hits 2786 2817 +31
+ Misses 938 905 -33
Continue to review full report at Codecov.
|
private.modulemap
Outdated
| @@ -0,0 +1,6 @@ | |||
| explicit module WireTransport.Private { | |||
There was a problem hiding this comment.
Nice solution to this problem!
We could also solve the issue with Swift interoperability - some of the ObjC classes have public and private headers. In Swift we can only access things in private header which makes it hard to extend ObjC classes in Swift because we need to have access to the same things as any other ObjC class.
The only thing that we should do here is to rename the module to WireTransport.Testing because this is not really private but test code.
There was a problem hiding this comment.
Nice, we should use it in other places too.
| @@ -678,6 +682,7 @@ | |||
| 54CA14FF1B32F2C1008D3787 /* ZMAccessToken.m */, | |||
| 54CA15001B32F2C1008D3787 /* ZMAccessTokenHandler.h */, | |||
| 54CA15011B32F2C1008D3787 /* ZMAccessTokenHandler.m */, | |||
| EF1A09A31FF6A73200F89634 /* ZMPersistentCookieStorage+Private.h */, | |||
There was a problem hiding this comment.
Same could be for the filename
| @@ -0,0 +1,29 @@ | |||
| //// | |||
| // Wire | |||
| // Copyright (C) 2017 Wire Swiss GmbH | |||
There was a problem hiding this comment.
| Your copyright header is so last year! |
| @@ -460,6 +463,7 @@ | |||
| 3E88BC9D1B1F35DF00232589 = { | |||
| isa = PBXGroup; | |||
| children = ( | |||
| EF1A09A51FF6BEE300F89634 /* private.modulemap */, | |||
There was a problem hiding this comment.
Sorry to bother you again, but this still has the old name :)
Diff with previous: 30.1.0...30.2.0 Commits: 9f93e6b zios-9245 remove keychain testing logic which prevent deleting items (#72)
What's new in this PR?
Issues
In ZMPersistentCookieStorage.deleteKeychainItems() method, keychain items may not be deleted if KeychainDisabled is set to true.
Causes
KeychainDisabled is enabled for when running tests only, which would not be used in the release. The test-only logic was not separated from the main logic.
Solutions
Remove KeychainDisabled check in ZMPersistentCookieStorage.deleteKeychainItems() (It is tested and would not affect the tests)
Move + (void)setDoNotPersistToKeychain:(BOOL)disabled to private header to confirm that it can only be used in tests.
Discussion
It would be better create a mock of ZMPersistentCookieStorage for running unit tests.