1249156 - Write unit tests for ClientID.jsm's getClientID method

Status: RESOLVED WORKSFORME

(Toolkit :: Telemetry, defect)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

In bug 1244295, we duplicate ClientID.jsm's getClientID method on Android in order to access the functionality before Gecko is running – we do this by reading the file directly off disk. We added some tests for the client ID java code (see GeckoProfile.getClientId & TestGeckoProfile) in bug 1244295 to help prevent regressions.

However, we should also write tests on the gecko side to further prevent regressions.

In particular, we should ensure the method writes and reads to the file in the expected format, to ensure it doesn't change under Java:
 {
  "clientID": "<client-ID>"
  ...
 }

Comment 1

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

The regression warning was added in bug 1244295.

Comment 2

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

While unit tests are ideal here, it will take a lot of time for me to 1) set up a desktop build to run the tests and 2) learn how to write tests for Gecko so I don't think it's worthwhile, given we have Java <-> JS client ID integration tests from bug 1249491.

Georg, do you agree? How necessary do you think this is?

Comment 3

[Georg Fritzsche [:gfritzsche]]

(In reply to Michael Comella (:mcomella) from comment #2)
> While unit tests are ideal here, it will take a lot of time for me to 1) set
> up a desktop build to run the tests and 2) learn how to write tests for
> Gecko so I don't think it's worthwhile, given we have Java <-> JS client ID
> integration tests from bug 1249491.
> 
> Georg, do you agree? How necessary do you think this is?

Do the test from bug 1249491 cover the "JS writes client id to disk, Java loads it successfully" scenario too?
It seems that is the only part left to worry about here.

Comment 4

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Georg Fritzsche [:gfritzsche] from comment #3)
> Do the test from bug 1249491 cover the "JS writes client id to disk, Java
> loads it successfully" scenario too?
> It seems that is the only part left to worry about here.

Yep! https://dxr.mozilla.org/mozilla-central/rev/c4449eab07d39e20ea315603f1b1863eeed7dcfe/mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/testUnifiedTelemetryClientId.java#96