Add unit testing for more org.bluez.Error.*

Add unit testing for org.bluez.Error.* translation

Added checks for correct translation:
  * Error.Failed
  * Error.ReadNotPermitted
  * Error.WriteNotPermitted
  * Error.NotSupported

Error.InProgress tested via new SetExtraProcessing
Error.NotAuthorized tested via new SetAuthorized
Error.NotPaired tested via new SetAuthenticated

BUG=377232
Committed: https://crrev.com/105e536840535f60cac1c5e852ecc892725067cc
Cr-Commit-Position: refs/heads/master@{#297752}

[Marie Janssen, 2014-10-01 18:23:28]

jamuraa@chromium.org changed reviewers:
+ armansito@chromium.org

[Marie Janssen, 2014-10-01 18:23:29]

Here's a method for testing NotAuthorized and InProgress.  I might update this
CL with tests for NotPaired and NotSupported but I'm unsure of the semantic
difference between NotSupported and (Read|Write)NotPermitted

[armansito, 2014-10-01 20:08:23]

https://codereview.chromium.org/619973002/diff/20001/chromeos/dbus/fake_bluet...
File chromeos/dbus/fake_bluetooth_gatt_characteristic_client.cc (right):

https://codereview.chromium.org/619973002/diff/20001/chromeos/dbus/fake_bluet...
chromeos/dbus/fake_bluetooth_gatt_characteristic_client.cc:31: return;
Return statement necessary?

https://codereview.chromium.org/619973002/diff/20001/chromeos/dbus/fake_bluet...
chromeos/dbus/fake_bluetooth_gatt_characteristic_client.cc:136:
error_callback.Run("org.bluez.Error.NotAuthorized", "Authenticate first");
"Authorize first"?

https://codereview.chromium.org/619973002/diff/20001/chromeos/dbus/fake_bluet...
chromeos/dbus/fake_bluetooth_gatt_characteristic_client.cc:154: struct
DelayedCallback* delayed = action_extra_requests_["ReadValue"];
struct keyword not necessary in C++

https://codereview.chromium.org/619973002/diff/20001/chromeos/dbus/fake_bluet...
chromeos/dbus/fake_bluetooth_gatt_characteristic_client.cc:401: for (const auto&
it : action_extra_requests_) {
Nice, I guess we should start adopting these C++11 idioms. That said, we should
try to make the rest of the code consistent with this, so maybe add a TODO to
use foreach loops elsewhere in this file?

https://codereview.chromium.org/619973002/diff/20001/chromeos/dbus/fake_bluet...
File chromeos/dbus/fake_bluetooth_gatt_characteristic_client.h (right):

https://codereview.chromium.org/619973002/diff/20001/chromeos/dbus/fake_bluet...
chromeos/dbus/fake_bluetooth_gatt_characteristic_client.h:163:
std::map<std::string, struct DelayedCallback*> action_extra_requests_;
struct keyword not necessary

https://codereview.chromium.org/619973002/diff/20001/device/bluetooth/bluetoo...
File device/bluetooth/bluetooth_gatt_chromeos_unittest.cc (right):

https://codereview.chromium.org/619973002/diff/20001/device/bluetooth/bluetoo...
device/bluetooth/bluetooth_gatt_chromeos_unittest.cc:972:
base::Unretained(this)));
Add new line before comments, here and below.

https://codereview.chromium.org/619973002/diff/20001/device/bluetooth/bluetoo...
device/bluetooth/bluetooth_gatt_chromeos_unittest.cc:985:
EXPECT_EQ(BluetoothGattService::GattErrorCode::GATT_ERROR_IN_PROGRESS,
Interesting, this works in C++? BluetoothGattService::GATT_ERROR_IN_PROGRESS
should be enough, or it's at least more consistent with the rest of the code.

[Marie Janssen, 2014-10-01 22:22:23]

PTAL, addressed comments and added tests for the rest of the dbus errors.

https://codereview.chromium.org/619973002/diff/20001/chromeos/dbus/fake_bluet...
File chromeos/dbus/fake_bluetooth_gatt_characteristic_client.cc (right):

https://codereview.chromium.org/619973002/diff/20001/chromeos/dbus/fake_bluet...
chromeos/dbus/fake_bluetooth_gatt_characteristic_client.cc:31: return;
On 2014/10/01 20:08:22, armansito wrote:
> Return statement necessary?

Hmm.  I swore I had issues with the Chromium about an explicit destructor until
I added it. Done.

https://codereview.chromium.org/619973002/diff/20001/chromeos/dbus/fake_bluet...
chromeos/dbus/fake_bluetooth_gatt_characteristic_client.cc:136:
error_callback.Run("org.bluez.Error.NotAuthorized", "Authenticate first");
On 2014/10/01 20:08:22, armansito wrote:
> "Authorize first"?

Done.

https://codereview.chromium.org/619973002/diff/20001/chromeos/dbus/fake_bluet...
chromeos/dbus/fake_bluetooth_gatt_characteristic_client.cc:154: struct
DelayedCallback* delayed = action_extra_requests_["ReadValue"];
On 2014/10/01 20:08:22, armansito wrote:
> struct keyword not necessary in C++

Done.

https://codereview.chromium.org/619973002/diff/20001/chromeos/dbus/fake_bluet...
chromeos/dbus/fake_bluetooth_gatt_characteristic_client.cc:401: for (const auto&
it : action_extra_requests_) {
On 2014/10/01 20:08:22, armansito wrote:
> Nice, I guess we should start adopting these C++11 idioms. That said, we
should
> try to make the rest of the code consistent with this, so maybe add a TODO to
> use foreach loops elsewhere in this file?

This is the only foreach loop in this file.

https://codereview.chromium.org/619973002/diff/20001/device/bluetooth/bluetoo...
File device/bluetooth/bluetooth_gatt_chromeos_unittest.cc (right):

https://codereview.chromium.org/619973002/diff/20001/device/bluetooth/bluetoo...
device/bluetooth/bluetooth_gatt_chromeos_unittest.cc:972:
base::Unretained(this)));
On 2014/10/01 20:08:22, armansito wrote:
> Add new line before comments, here and below.

Done.

https://codereview.chromium.org/619973002/diff/20001/device/bluetooth/bluetoo...
device/bluetooth/bluetooth_gatt_chromeos_unittest.cc:985:
EXPECT_EQ(BluetoothGattService::GattErrorCode::GATT_ERROR_IN_PROGRESS,
On 2014/10/01 20:08:22, armansito wrote:
> Interesting, this works in C++? BluetoothGattService::GATT_ERROR_IN_PROGRESS
> should be enough, or it's at least more consistent with the rest of the code.

Done.

[armansito, 2014-10-01 22:25:54]

lgtm with one comment

https://codereview.chromium.org/619973002/diff/40001/chromeos/dbus/fake_bluet...
File chromeos/dbus/fake_bluetooth_gatt_characteristic_client.h (right):

https://codereview.chromium.org/619973002/diff/40001/chromeos/dbus/fake_bluet...
chromeos/dbus/fake_bluetooth_gatt_characteristic_client.h:173:
std::map<std::string, DelayedCallback*> action_extra_requests_;
Make sure to delete these in the destructor.

[Marie Janssen, 2014-10-02 00:19:58]

The CQ bit was checked by jamuraa@chromium.org

[commit-bot: I haz the power, 2014-10-02 00:20:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/619973002/60001

[commit-bot: I haz the power, 2014-10-02 00:56:46]

Committed patchset #4 (id:60001) as 9a331b544490f1117e2cda3e30a56fe5fe81cd9a

[commit-bot: I haz the power, 2014-10-02 00:57:06]

Patchset 4 (id:??) landed as
https://crrev.com/105e536840535f60cac1c5e852ecc892725067cc
Cr-Commit-Position: refs/heads/master@{#297752}