Bug 1379316 - Move test with broken telemetry init out of test_TelemetrySession.js. r?chutten draft
authorAlessio Placitelli <alessio.placitelli@gmail.com>
Thu, 24 Aug 2017 18:22:04 +0200
changeset 652877 804b2a4110a3a06fc9f2b7bcd1fcb1a96f043299
parent 652136 892c8916ba32b7733e06bfbfdd4083ffae3ca028
child 728208 16971bdfaaf417d138603b20b87853add3f32dd6
push id76183
push useralessio.placitelli@gmail.com
push dateFri, 25 Aug 2017 09:13:20 +0000
reviewerschutten
bugs1379316
milestone57.0a1
Bug 1379316 - Move test with broken telemetry init out of test_TelemetrySession.js. r?chutten Sometimes, test receive pings that should not be there. This happens because Telemetry is initialized multiple times by some tests: this patch fixes one test and moves another flaky test to a separate file. Additionally, add useful debug information to the logs to understand which ping made the test fail. MozReview-Commit-ID: BS25U3e9fxO
toolkit/components/telemetry/tests/unit/test_TelemetrySession.js
toolkit/components/telemetry/tests/unit/test_TelemetrySession_abortedSessionQueued.js
toolkit/components/telemetry/tests/unit/xpcshell.ini
--- a/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js
+++ b/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js
@@ -1712,39 +1712,16 @@ add_task(async function test_abortedSess
   checkPingFormat(updatedAbortedSessionPing, PING_TYPE_MAIN, true, true);
   Assert.equal(updatedAbortedSessionPing.payload.info.reason, REASON_ABORTED_SESSION);
   Assert.notEqual(abortedSessionPing.id, updatedAbortedSessionPing.id);
   Assert.notEqual(abortedSessionPing.creationDate, updatedAbortedSessionPing.creationDate);
 
   await TelemetryController.testShutdown();
   Assert.ok(!(await OS.File.exists(ABORTED_FILE)),
             "No aborted session ping must be available after a shutdown.");
-
-  // Write the ping to the aborted-session file. TelemetrySession will add it to the
-  // saved pings directory when it starts.
-  await TelemetryStorage.savePingToFile(abortedSessionPing, ABORTED_FILE, false);
-  Assert.ok((await OS.File.exists(ABORTED_FILE)),
-            "The aborted session ping must exist in the aborted session ping directory.");
-
-  await TelemetryStorage.testClearPendingPings();
-  PingServer.clearRequests();
-  await TelemetryController.testReset();
-
-  Assert.ok(!(await OS.File.exists(ABORTED_FILE)),
-            "The aborted session ping must be removed from the aborted session ping directory.");
-
-  // Restarting Telemetry again to trigger sending pings in TelemetrySend.
-  await TelemetryController.testReset();
-
-  // We should have received an aborted-session ping.
-  const receivedPing = await PingServer.promiseNextPing();
-  Assert.equal(receivedPing.type, PING_TYPE_MAIN, "Should have the correct type");
-  Assert.equal(receivedPing.payload.info.reason, REASON_ABORTED_SESSION, "Ping should have the correct reason");
-
-  await TelemetryController.testShutdown();
 });
 
 add_task(async function test_abortedSession_Shutdown() {
   if (gIsAndroid) {
     // We don't have the aborted session ping here.
     return;
   }
 
@@ -1831,18 +1808,19 @@ add_task(async function test_abortedDail
 add_task(async function test_schedulerComputerSleep() {
   if (gIsAndroid) {
     // We don't have the aborted session or the daily ping here.
     return;
   }
 
   const ABORTED_FILE = OS.Path.join(DATAREPORTING_PATH, ABORTED_PING_FILE_NAME);
 
+  await TelemetryController.testReset();
+  await TelemetryController.testShutdown();
   await TelemetryStorage.testClearPendingPings();
-  await TelemetryController.testReset();
   PingServer.clearRequests();
 
   // Remove any aborted-session ping from the previous tests.
   await OS.File.removeDir(DATAREPORTING_PATH, { ignoreAbsent: true });
 
   // Set a fake current date and start Telemetry.
   let nowDate = fakeNow(2009, 10, 18, 0, 0, 0);
   let schedulerTickCallback = null;
@@ -1918,17 +1896,18 @@ add_task(async function test_schedulerEn
   // Trigger the environment change.
   Preferences.set(PREF_TEST, 1);
 
   // Wait for the environment-changed ping.
   await PingServer.promiseNextPing();
 
   // We don't expect to receive any daily ping in this test, so assert if we do.
   PingServer.registerPingHandler((req, res) => {
-    Assert.ok(false, "No ping should be sent/received in this test.");
+    const receivedPing = decodeRequestPayload(req);
+    Assert.ok(false, `No ping should be received in this test (got ${receivedPing.id}).`);
   });
 
   // Execute one scheduler tick. It should not trigger a daily ping.
   Assert.ok(!!schedulerTickCallback);
   await schedulerTickCallback();
   await TelemetryController.testShutdown();
 });
 
@@ -1942,17 +1921,18 @@ add_task(async function test_schedulerNo
 
   // Remove any aborted-session ping from the previous tests.
   await OS.File.removeDir(DATAREPORTING_PATH, { ignoreAbsent: true });
   await TelemetryStorage.testClearPendingPings();
   await TelemetryController.testReset();
 
   // We don't expect to receive any ping in this test, so assert if we do.
   PingServer.registerPingHandler((req, res) => {
-    Assert.ok(false, "No ping should be sent/received in this test.");
+    const receivedPing = decodeRequestPayload(req);
+    Assert.ok(false, `No ping should be received in this test (got ${receivedPing.id}).`);
   });
 
   // Set a current date/time away from midnight, so that the daily ping doesn't get
   // sent.
   let nowDate = new Date(2009, 10, 18, 11, 0, 0);
   fakeNow(nowDate);
   let schedulerTickCallback = null;
   fakeSchedulerTimer(callback => schedulerTickCallback = callback, () => {});
new file mode 100644
--- /dev/null
+++ b/toolkit/components/telemetry/tests/unit/test_TelemetrySession_abortedSessionQueued.js
@@ -0,0 +1,79 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/
+*/
+
+/**
+ * This file only contains the |test_abortedSessionQueued| test. This needs
+ * to be in a separate, stand-alone file since we're initializing Telemetry
+ * twice, in a non-standard way to simulate incorrect shutdowns. Doing this
+ * in other files might interfere with the other tests.
+ */
+
+Cu.import("resource://gre/modules/TelemetryStorage.jsm", this);
+Cu.import("resource://gre/modules/Services.jsm", this);
+
+const DATAREPORTING_DIR = "datareporting";
+const ABORTED_PING_FILE_NAME = "aborted-session-ping";
+const ABORTED_SESSION_UPDATE_INTERVAL_MS = 5 * 60 * 1000;
+
+const PING_TYPE_MAIN = "main";
+const REASON_ABORTED_SESSION = "aborted-session";
+
+XPCOMUtils.defineLazyGetter(this, "DATAREPORTING_PATH", function() {
+  return OS.Path.join(OS.Constants.Path.profileDir, DATAREPORTING_DIR);
+});
+
+add_task(async function test_setup() {
+  do_get_profile();
+  PingServer.start();
+  Services.prefs.setCharPref(TelemetryUtils.Preferences.Server, "http://localhost:" + PingServer.port);
+});
+
+add_task(async function test_abortedSessionQueued() {
+  const ABORTED_FILE = OS.Path.join(DATAREPORTING_PATH, ABORTED_PING_FILE_NAME);
+
+  // Make sure the aborted sessions directory does not exist to test its creation.
+  await OS.File.removeDir(DATAREPORTING_PATH, { ignoreAbsent: true });
+
+  let schedulerTickCallback = null;
+  let now = new Date(2040, 1, 1, 0, 0, 0);
+  fakeNow(now);
+  // Fake scheduler functions to control aborted-session flow in tests.
+  fakeSchedulerTimer(callback => schedulerTickCallback = callback, () => {});
+  await TelemetryController.testReset();
+
+  Assert.ok((await OS.File.exists(DATAREPORTING_PATH)),
+            "Telemetry must create the aborted session directory when starting.");
+
+  // Fake now again so that the scheduled aborted-session save takes place.
+  now = futureDate(now, ABORTED_SESSION_UPDATE_INTERVAL_MS);
+  fakeNow(now);
+  // The first aborted session checkpoint must take place right after the initialisation.
+  Assert.ok(!!schedulerTickCallback);
+  // Execute one scheduler tick.
+  await schedulerTickCallback();
+  // Check that the aborted session is due at the correct time.
+  Assert.ok((await OS.File.exists(ABORTED_FILE)),
+            "There must be an aborted session ping.");
+
+  await TelemetryStorage.testClearPendingPings();
+  PingServer.clearRequests();
+  await TelemetryController.testReset();
+
+  Assert.ok(!(await OS.File.exists(ABORTED_FILE)),
+            "The aborted session ping must be removed from the aborted session ping directory.");
+
+  // Restarting Telemetry again to trigger sending pings in TelemetrySend.
+  await TelemetryController.testReset();
+
+  // We should have received an aborted-session ping.
+  const receivedPing = await PingServer.promiseNextPing();
+  Assert.equal(receivedPing.type, PING_TYPE_MAIN, "Should have the correct type");
+  Assert.equal(receivedPing.payload.info.reason, REASON_ABORTED_SESSION, "Ping should have the correct reason");
+
+  await TelemetryController.testShutdown();
+});
+
+add_task(async function stopServer() {
+  await PingServer.stop();
+});
--- a/toolkit/components/telemetry/tests/unit/xpcshell.ini
+++ b/toolkit/components/telemetry/tests/unit/xpcshell.ini
@@ -46,16 +46,18 @@ tags = addons
 tags = addons
 [test_TelemetryStopwatch.js]
 [test_TelemetryControllerBuildID.js]
 [test_TelemetrySendOldPings.js]
 skip-if = os == "android" # Disabled due to intermittent orange on Android
 tags = addons
 [test_TelemetrySession.js]
 tags = addons
+[test_TelemetrySession_abortedSessionQueued.js]
+skip-if = os == "android"
 [test_TelemetrySession_activeTicks.js]
 [test_TelemetrySend.js]
 [test_ChildHistograms.js]
 skip-if = os == "android" # Disabled due to crashes (see bug 1331366)
 tags = addons
 [test_ChildScalars.js]
 skip-if = os == "android" # Disabled due to crashes (see bug 1331366)
 [test_TelemetryReportingPolicy.js]