943263 - B2G MMS : add test cases for MobileMessageDB::forEachMatchedMmsDeliveryInfo

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[Vicamo Yang [:vicamo][:vyang]]

+++ This bug was initially created as a clone of Bug #942780 +++

To unblock bug 942780 from bug 940884, move test cases to a new bug and depend on them both.

Comment 1

[Vicamo Yang [:vicamo][:vyang]]

Attachment: patch

Comment 2

[Gene Lian (I already quit Mozilla)]

Comment on attachment 8338431 [details] [diff] [review]
patch

Review of attachment 8338431 [details] [diff] [review]:
-----------------------------------------------------------------

How nice!

::: dom/mobilemessage/tests/marionette/test_mmdb_foreachmatchedmmsdeliveryinfo.js
@@ +38,5 @@
> +
> +  let count = 0;
> +  aMmdb.forEachMatchedMmsDeliveryInfo(deliveryInfo, aNeedle, function(aElement) {
> +    ok(true, "checking " + aElement.receiver);
> +    ok(!aElement.hasOwnProperty("traversed"), "element.traversed");

I don't think need this line since you just called:

clearTraversed(deliveryInfo);

This sounds a bit redundant to me.

::: dom/mobilemessage/tests/marionette/test_mmdb_new.js
@@ +7,5 @@
>  startTestBase(function testCaseMain() {
>    log("Test init MobileMessageDB");
>  
> +  // TODO: bug 943233 - passing jsm exported objects to |Promise.resolve| gets
> +  // empty object in return.

The coding style for comment should be:

// {upper_case_letter}...{period}

so, s/e/E

Comment 3

[Gene Lian (I already quit Mozilla)]

Sorry, I misunderstood for the last comment. Don't need to s/e/E because it's a new line.

Comment 4

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Gene Lian [:gene] (needinfo? encouraged) from comment #2)
> :::
> dom/mobilemessage/tests/marionette/test_mmdb_foreachmatchedmmsdeliveryinfo.js
> @@ +38,5 @@
> > +
> > +  let count = 0;
> > +  aMmdb.forEachMatchedMmsDeliveryInfo(deliveryInfo, aNeedle, function(aElement) {
> > +    ok(true, "checking " + aElement.receiver);
> > +    ok(!aElement.hasOwnProperty("traversed"), "element.traversed");
> 
> I don't think need this line since you just called:
> clearTraversed(deliveryInfo);
> This sounds a bit redundant to me.

The reason there is a function called |clearTraversed| is to check any of the elements is not traversed twice.  So somehow there must be someway to mark/check/clean a certain attribute, or we should never have it at all.  If you still have other concern ...

Comment 5

[Gene Lian (I already quit Mozilla)]

OK. It's fine.

Comment 6

[Vicamo Yang [:vicamo][:vyang]]

https://hg.mozilla.org/integration/b2g-inbound/rev/0d19e7cd7665

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0d19e7cd7665