Closed Bug 750948 Opened 9 years ago Closed 9 years ago

Implement unit tests for AitC client against mock server

Categories

(Web Apps Graveyard :: AppsInTheCloud, defect)

defect
Not set
normal

Tracking

(blocking-kilimanjaro:-)

RESOLVED FIXED
blocking-kilimanjaro -

People

(Reporter: anant, Assigned: nick)

References

Details

(Whiteboard: [blocking-aitc+][qa-])

Attachments

(1 file, 2 obsolete files)

The mock server for AitC is being written in 749336, we need unit tests that run against it.
- Test PUTting an app on the server
- Test GETting a list of apps, and proper receipt of 304 status codes
- Test if the client obeys X-Backoff and Retry-After headers
- Simulate error conditions and verify client behavior:
-- Client installs an app, AITC is down
-- Client installs an app, PUT never reaches server
-- Client installs an app, PUT reaches server but response is lost
Duplicate of this bug: 745002
blocking-kilimanjaro: --- → ?
More test ideas:

- Add an app locally, make sure it gets to the server
- Delete app locally, check server
- Update an app on the server, make sure the client gets the update
- Update an app locally, make sure the server gets the update
- Delete an app on the server, make sure client gets the delete
- Update an app locally, delete on server, to create a conflict (not sure how it should be resolved, but it's a case ;)
- Update an app on the server, update locally, connect to server and get it to resolve the conflict
- Force some cases like losing the last modified time, and make sure the client handles that gracefully (like it might do a larger check than necessary, but it should still do so successfully).  Any other state that is saved could be deleted and checked similarly.
- Put an app on the server that is invalid, make sure client doesn't die terribly (this might be hard on the AitC server, which does validation, but probably easy on the mock server)
- Simulate a few authentication errors (not sure how many stages of auth there are); errors should be reported back somewhere.
(In reply to Ian Bicking (:ianb) from comment #3)
> More test ideas:
> 
> - Add an app locally, make sure it gets to the server
> - Delete app locally, check server
> - Update an app on the server, make sure the client gets the update
> - Update an app locally, make sure the server gets the update
> - Delete an app on the server, make sure client gets the delete
> - Update an app locally, delete on server, to create a conflict (not sure
> how it should be resolved, but it's a case ;)
> - Update an app on the server, update locally, connect to server and get it
> to resolve the conflict
> - Force some cases like losing the last modified time, and make sure the
> client handles that gracefully (like it might do a larger check than
> necessary, but it should still do so successfully).  Any other state that is
> saved could be deleted and checked similarly.
> - Put an app on the server that is invalid, make sure client doesn't die
> terribly (this might be hard on the AitC server, which does validation, but
> probably easy on the mock server)
> - Simulate a few authentication errors (not sure how many stages of auth
> there are); errors should be reported back somewhere.

Good ideas. Probably should get this documented in a test plan somewhere, along with how it maps between manual test cases --> unit test automation --> functional test automation. Any preference on a wiki location maybe for this? Maybe something under https://wiki.mozilla.org/Apps/AITC listed as Client Test Plan?
OS: Mac OS X → All
Hardware: x86 → All
Nice cases!

(In reply to Ian Bicking (:ianb) from comment #3)
> - Update an app locally, delete on server, to create a conflict (not sure
> how it should be resolved, but it's a case ;)

In the current implementation, server always wins. I believe that's alright :)

> - Update an app on the server, update locally, connect to server and get it
> to resolve the conflict

The client should (hopefully) be comparing lastModified values to decide who wins.
Blocks: 744257
blocking-kilimanjaro: ? → -
Depends on: 749336
Blocks: 754538
No longer blocks: 744257
Whiteboard: [blocking-aitc]
Whiteboard: [blocking-aitc] → [blocking-aitc+]
Are there any more bugs that are blocking this? If not, let's land it! Harald, Greg?
Status: NEW → ASSIGNED
Initial draft for mock-server tests, want to sign off the general way the mock-server is used. Will eventually need more test cases.
Attachment #634070 - Flags: review?
Attachment #634070 - Flags: feedback?(gps)
Attachment #634070 - Flags: feedback?(anant)
Comment on attachment 634070 [details] [diff] [review]
Mock-server-driven AITC client tests.

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

Tests \o/

::: services/aitc/tests/unit/test_aitc_client.js
@@ +5,5 @@
> +
> +Cu.import("resource://services-common/rest.js");
> +Cu.import("resource://services-common/utils.js");
> +Cu.import("resource://services-common/preferences.js");
> +Cu.import("resource://services-aitc/client.js");

Depending on when this lands, you may need to Cu.import("resource://testing-common/services-common/aitcserver.js"); This change *may* land in the next week. You may want to run it by me before this is committed, just in case.

@@ +34,5 @@
> +}
> +
> +function get_mock_app(remote) {
> +
> +  var app = {

Always use let.

@@ +43,5 @@
> +    modifiedAt: Date.now(),
> +    receipts: []
> +  };
> +
> +  app[(remote ? 'manifestPath' : 'manifestURL')] = "/manifest.webapp";

Do you need the parenthesis?

@@ +72,5 @@
> +  let client = get_client_for_server(username, server);
> +
> +  client.getApps(function(error, apps) {
> +    _("Got response");
> +    do_check_eq(error, null);

do_check_null. Fix other instances as well.
Attachment #634070 - Flags: feedback?(gps) → feedback+
Incorporated feedback. I will extend test coverage after verifying on how to test backoff and other timeout-dependent behaviors.
Attachment #634070 - Attachment is obsolete: true
Attachment #634070 - Flags: review?
Attachment #634070 - Flags: feedback?(anant)
Attachment #636509 - Flags: review?(gps)
Comment on attachment 636509 [details] [diff] [review]
Mock-server-driven AITC client tests.

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

General nit: use double quotes for all string literals.

Other than that, looks good.

I just cut a services-central release train for QA. This means we shouldn't land things in services-central until QA has had a chance to sign off on them. But, I don't think they are going to start testing until tomorrow, so there's time to roll another build. If you land this in the next few hours, it shouldn't be an issue. Else, hold off for the next train or land in inbound.
Attachment #636509 - Flags: review?(gps) → review+
What's blocking this from landing btw?
(In reply to Jason Smith [:jsmith] from comment #11)
> What's blocking this from landing btw?

Just the style nits. I'll check in a new patch if one is uploaded.
Anant and I noticed this patch bitrot, and therefor we updated it to work again.
Assignee: hkirschner → ndesaulniers
Attachment #636509 - Attachment is obsolete: true
Attachment #641643 - Flags: review?(gps)
Attachment #641643 - Flags: review?(gps) → review+
https://hg.mozilla.org/services/services-central/rev/a2fe3cbf454a
Whiteboard: [blocking-aitc+] → [blocking-aitc+][fixed in services]
Whiteboard: [blocking-aitc+][fixed in services] → [blocking-aitc+], [fixed in services], [qa-]
https://hg.mozilla.org/mozilla-central/rev/a2fe3cbf454a
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [blocking-aitc+], [fixed in services], [qa-] → [blocking-aitc+][qa-]
Product: Web Apps → Web Apps Graveyard
You need to log in before you can comment on or make changes to this bug.