Provide unittests for Obj-C API IdentityManagerObserverBridge |
||
Issue descriptionWhile working on crbug.com/903262 I noticed an error in the implementation of IdentityManagerObserverBridge that forced me to provide a fix for it first (see crbug.com/903391 ), and made me think that this wouldn't have happened if there was a test suite for this new API, so here is a bug to track it down. CCing @jlebel and @blundell as the author and reviewer of the CL that landed this new API in https://chromium-review.googlesource.com/c/chromium/src/+/1203934
,
Nov 9
Agreed, unittests would be great here (and I should have asked for them on the initial review). Jerome, I think that it makes the most sense for you to look at that whenever you have some cycles. Thanks! |
||
►
Sign in to add a comment |
||
Comment 1 by ma...@igalia.com
, Nov 8@blundell @jlebel Even if it's not much, I'm pasting here the WIP skeleton I wrote today while spending some time to figure out how this could be done, in case it's useful: diff --git a/services/identity/BUILD.gn b/services/identity/BUILD.gn index 435ece170e26..4e1bca45b65c 100644 --- a/services/identity/BUILD.gn +++ b/services/identity/BUILD.gn @@ -59,6 +59,12 @@ source_set("tests") { "public/cpp/identity_test_environment_unittest.cc", "public/cpp/primary_account_access_token_fetcher_unittest.cc", ] + + if (is_mac || is_ios) { + configs += [ "//build/config/compiler:enable_arc" ] + deps += [ "//services/identity/public/objc" ] + sources += [ "public/objc/identity_manager_observer_bridge_unittest.mm" ] + } } service_manifest("unittest_manifest") { diff --git a/services/identity/public/objc/identity_manager_observer_bridge_unittest.mm b/services/identity/public/objc/identity_manager_observer_bridge_unittest.mm new file mode 100644 index 000000000000..e86c8376f8fb --- /dev/null +++ b/services/identity/public/objc/identity_manager_observer_bridge_unittest.mm @@ -0,0 +1,20 @@ +// Copyright 2018 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "services/identity/public/objc/identity_manager_observer_bridge.h" + +#include "testing/gtest/include/gtest/gtest.h" +#include "testing/platform_test.h" + +#if !defined(__has_feature) || !__has_feature(objc_arc) +#error "This file requires ARC support." +#endif + +namespace identity { + +using IdentityManagerObserverBridgeTest = PlatformTest; + +TEST_F(IdentityManagerObserverBridgeTest, FooBar) {} + +} // namespace identity @blundell While initially I thought I could do this as part of the fix for crbug.com/903262 , I'm not that sure now since it might make more sense for @jlebel to do it since he's the one that implemented the API in the first place, so that's why I haven't labelled this ticket as Proj-Servicification-VendorBug. If, however, you prefer me to spend some time to finish the skeleton posted above, let me know and I'll work on it, but I'd rather ask before diving into that while there are still more VendorBug tickets available :-)