Domain Reliability: Make context upload test cases less brittle

Domain Reliability: Make context upload test cases less brittle

Individually examine the fields of the uploaded beacon instead of
comparing the JSONified report to a static test string.

BUG=

Committed: https://crrev.com/fac61121919a4bab97323b10cff66d3a4cf6f232
Cr-Commit-Position: refs/heads/master@{#364401}

[Deprecated (see juliatuttle), 2015-12-09 20:50:52]

This is failing to compile with:

../../components/domain_reliability/context_unittest.cc:55:29: error: called
object type 'bool (base::DictionaryValue::*)(const std::string &, std::string *)
const' is not a function or function pointer
    bool got_value = dict->*GetValueType(key, &actual_value);
                            ^~~~~~~~~~~~
../../components/domain_reliability/context_unittest.cc:256:29: note: in
instantiation of member function 'domain_reliability::(anonymous
namespace)::HasValue<std::basic_string<char, std::char_traits<char>,
std::allocator<char> >, &base::DictionaryValue::GetString>::operator()'
requested here
  EXPECT_TRUE(HasStringValue(entry, "failure_data.custom_error",
                            ^

[Deprecated (see juliatuttle), 2015-12-09 20:52:01]

ttuttle@chromium.org changed reviewers:
+ jkarlin@chromium.org

[Deprecated (see juliatuttle), 2015-12-09 20:52:01]

PTAL, jkarlin.

[Deprecated (see juliatuttle), 2015-12-09 20:54:32]

On 2015/12/09 20:50:52, ttuttle wrote:
> This is failing to compile with:
> 
> ../../components/domain_reliability/context_unittest.cc:55:29: error: called
> object type 'bool (base::DictionaryValue::*)(const std::string &, std::string
*)
> const' is not a function or function pointer
>     bool got_value = dict->*GetValueType(key, &actual_value);
>                             ^~~~~~~~~~~~
> ../../components/domain_reliability/context_unittest.cc:256:29: note: in
> instantiation of member function 'domain_reliability::(anonymous
> namespace)::HasValue<std::basic_string<char, std::char_traits<char>,
> std::allocator<char> >, &base::DictionaryValue::GetString>::operator()'
> requested here
>   EXPECT_TRUE(HasStringValue(entry, "failure_data.custom_error",
>                             ^

Apparently I needed parens around (dict->*GetValueType), so it parses it as
"take this method, call it on dict, with these args" instead of "take this
function pointer, call it with these args, and then... use that to derefence
dict".

[jkarlin, 2015-12-10 12:49:54]

lgtm with minor changes

https://codereview.chromium.org/1511243003/diff/40001/components/domain_relia...
File components/domain_reliability/context_unittest.cc (right):

https://codereview.chromium.org/1511243003/diff/40001/components/domain_relia...
components/domain_reliability/context_unittest.cc:51: bool operator()(const
DictionaryValue* dict,
const DictionaryValue& dict

https://codereview.chromium.org/1511243003/diff/40001/components/domain_relia...
components/domain_reliability/context_unittest.cc:67: bool
GetEntryFromReport(const Value* report,
const Value& report

[Deprecated (see juliatuttle), 2015-12-10 16:00:04]

PTAL, jkarlin.

https://codereview.chromium.org/1511243003/diff/40001/components/domain_relia...
File components/domain_reliability/context_unittest.cc (right):

https://codereview.chromium.org/1511243003/diff/40001/components/domain_relia...
components/domain_reliability/context_unittest.cc:51: bool operator()(const
DictionaryValue* dict,
On 2015/12/10 12:49:54, jkarlin wrote:
> const DictionaryValue& dict

Done.

https://codereview.chromium.org/1511243003/diff/40001/components/domain_relia...
components/domain_reliability/context_unittest.cc:67: bool
GetEntryFromReport(const Value* report,
On 2015/12/10 12:49:54, jkarlin wrote:
> const Value& report

I wanted to keep this as a pointer so it can do the null check for that pointer
too. If I don't, it'd mean an extra line in each test case.

[jkarlin, 2015-12-10 16:04:35]

slgtm

https://codereview.chromium.org/1511243003/diff/40001/components/domain_relia...
File components/domain_reliability/context_unittest.cc (right):

https://codereview.chromium.org/1511243003/diff/40001/components/domain_relia...
components/domain_reliability/context_unittest.cc:67: bool
GetEntryFromReport(const Value* report,
On 2015/12/10 16:00:04, ttuttle wrote:
> On 2015/12/10 12:49:54, jkarlin wrote:
> > const Value& report
> 
> I wanted to keep this as a pointer so it can do the null check for that
pointer
> too. If I don't, it'd mean an extra line in each test case.

sgtm

[Deprecated (see juliatuttle), 2015-12-10 18:03:53]

The CQ bit was checked by ttuttle@chromium.org

[Deprecated (see juliatuttle), 2015-12-10 18:03:54]

The patchset sent to the CQ was uploaded after l-g-t-m from jkarlin@chromium.org
Link to the patchset: https://codereview.chromium.org/1511243003/#ps60001
(title: "Make some requested changes")

[commit-bot: I haz the power, 2015-12-10 18:06:17]

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

[commit-bot: I haz the power, 2015-12-10 18:13:00]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-12-10 18:13:37]

Description was changed from

==========
Domain Reliability: Make context upload test cases less brittle

Individually examine the fields of the uploaded beacon instead of
comparing the JSONified report to a static test string.

BUG=
==========

to

==========
Domain Reliability: Make context upload test cases less brittle

Individually examine the fields of the uploaded beacon instead of
comparing the JSONified report to a static test string.

BUG=

Committed: https://crrev.com/fac61121919a4bab97323b10cff66d3a4cf6f232
Cr-Commit-Position: refs/heads/master@{#364401}
==========

[commit-bot: I haz the power, 2015-12-10 18:13:38]

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