Closed Bug 702717 Opened 10 years ago Closed 10 years ago

finalize the statements in services/sync/tests/unit/test_async_querySpinningly.js

Categories

(Firefox :: Sync, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla11

People

(Reporter: espindola, Assigned: espindola)

References

Details

(Whiteboard: [fixed in services][qa-])

Attachments

(1 file, 1 obsolete file)

No description provided.
Comment on attachment 574642 [details] [diff] [review]
finalize the statements in services/sync/tests/unit/test_async_querySpinningly.js

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

a Sync peer should review this
Attachment #574642 - Flags: review?(mak77) → review?(rnewman)
OS: Mac OS X → All
Hardware: x86 → All
Comment on attachment 574642 [details] [diff] [review]
finalize the statements in services/sync/tests/unit/test_async_querySpinningly.js

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

Don't forget to change the r= when you submit the revised version.

Thanks for tackling this!

::: services/sync/tests/unit/test_async_querySpinningly.js
@@ +6,5 @@
>  function run_test() {
>    initTestLogging("Trace");
>  
>    _("Using the form service to test queries");
>    function c(query) Svc.Form.DBConnection.createStatement(query);

While we're here, let's kill this and inline it.

@@ +7,5 @@
>    initTestLogging("Trace");
>  
>    _("Using the form service to test queries");
>    function c(query) Svc.Form.DBConnection.createStatement(query);
> +  function d(query) {

Let's call this "querySpinningly". That's all it does, after all.

@@ +9,5 @@
>    _("Using the form service to test queries");
>    function c(query) Svc.Form.DBConnection.createStatement(query);
> +  function d(query) {
> +    let q = c(query);
> +    let r

Missing ;, but let's just rewrite this:

  function querySpinningly(query, names) {
    let q = Svc.Form.DBConnection.createStatement(query);
    let r = Async.querySpinningly(q, names);
    q.finalize();
    return r;
  }

@@ +78,4 @@
>    do_check_eq(r10.length, 3);
>  
>    _("Generate an execution error");
>    let r11, except, query = c("INSERT INTO moz_formhistory (fieldname, value) VALUES ('one', NULL)");

We can just unpack the old definition of `c` here, and put in an additional `let` line.
Attachment #574642 - Flags: review?(rnewman) → feedback+
I won't have my ssh key with me until Saturday. Would you mind pushing to try and mozilla-inbound?

Thanks!
Attachment #574642 - Attachment is obsolete: true
Attachment #575266 - Flags: review?(rnewman)
(In reply to Rafael Ávila de Espíndola (:espindola) from comment #4)

> I won't have my ssh key with me until Saturday. Would you mind pushing to
> try and mozilla-inbound?

I cleaned this up some, fixed the patch headers, and pushed to s-c:

https://hg.mozilla.org/services/services-central/rev/dfc7ddc23451

(No need for a try build: it's a test-only change.)

Thanks for the patch! It'll get merged into m-c on Monday.
Whiteboard: [fixed in services][qa-]
https://hg.mozilla.org/mozilla-central/rev/dfc7ddc23451
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
Attachment #575266 - Flags: review?(rnewman)
Component: Firefox Sync: Backend → Sync
Product: Cloud Services → Firefox
You need to log in before you can comment on or make changes to this bug.