644288 - Make HashCompleter tests

Status: RESOLVED FIXED

(Toolkit :: Safe Browsing, defect)

Description

[Mehdi Mulani [:mmm] (I don't check this)]

As part of an overall goal to port HashCompleter to JS, it would be nice to have tests for the HashCompleter interface to ensure that we don't mess anything up as we bring it to JS.

Comment 1

[Mehdi Mulani [:mmm] (I don't check this)]

Attachment: WIP tests.

Looks like I finally have all the functionality that I want to test. Still missing some bits but they don't seem essential.

Comment 2

[Mehdi Mulani [:mmm] (I don't check this)]

Attachment: Tests for HashCompleter.

This tests a few parts but leaves some out.
Tests:
 - HashCompleter getting a 204 response
 - HashCompleter handling duplicate responses
 - multiple responses for the same hash prefix
 - random data (to test large requests)

Stuff not tested:
 - multiple requests at the same time
 - backoff rate correctly being handled by the spec
 - requests with a MAC

As for multiple requests and the backoff rate, it looks like we could later add a test for multiple requests and if we exposed some data related to the next scheduled request, perhaps add a test for the backoff rate.

I'm not completely sure how to test requests with a MAC mainly because I haven't worked on re-implementing that bit yet. Are MACs even used with the current safebrowsing?

Comment 3

[Mehdi Mulani [:mmm] (I don't check this)]

Attachment: Correct version of tests.

Sorry, forgot to refresh before diff'ing. Confirmed that this is the correct version I want reviewed :)

Comment 4

[Dave Camp (:dcamp)]

Comment on attachment 523213 [details] [diff] [review]
Correct version of tests.

Looks good.

Eventually you might want to include some tests of the client key interaction.  test_streamupdater.js has some hmac stuff you should be able to crib from.

Comment 5

[Zack Weinberg (:zwol)]

http://hg.mozilla.org/mozilla-central/rev/e88652085a4e

Comment 6

[Mehdi Mulani [:mmm] (I don't check this)]

An intermittent orange has shown up with this test. Bug 652294