From 6f9276855969e02091622c0e25ccdc06bbb43232 Mon Sep 17 00:00:00 2001
From: Emily Stark <emily@meteor.com>
Date: Wed, 18 Jun 2014 19:47:23 -0700
Subject: [PATCH 01/10] Add hostname/meteor release/os to package stats

---
 tools/stats.js              | 21 ++++++++++++++++++++-
 tools/tests/report-stats.js | 18 ++++++++++++++++++
 2 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/tools/stats.js b/tools/stats.js
index db925a60b7..56ccef6026 100644
--- a/tools/stats.js
+++ b/tools/stats.js
@@ -1,5 +1,6 @@
 var Fiber = require("fibers");
 var _ = require("underscore");
+var os = require("os");
 
 var config = require("./config.js");
 var files = require("./files.js");
@@ -7,6 +8,7 @@ var uniload = require("./uniload.js");
 var project = require("./project.js");
 var auth = require("./auth.js");
 var ServiceConnection = require("./service-connection.js");
+var release = require("./release.js");
 
 // The name of the package that you add to your app to opt out of
 // sending stats.
@@ -43,6 +45,22 @@ var recordPackages = function () {
   // to the package stats server. If we can't connect, for example, we
   // don't care; we'll just miss out on recording these packages.
   Fiber(function () {
+
+    var userAgentInfo = {
+      hostname: os.hostname(),
+      osPlatform: os.platform(),
+      osType: os.type(),
+      osRelease: os.release(),
+      osArch: os.arch()
+    };
+
+    if (! release.current.isCheckout()) {
+      userAgentInfo.meteorReleaseTrack = release.getReleaseTrack();
+      userAgentInfo.meteorReleaseVersion = release.getReleaseVersion();
+      userAgentInfo.meteorToolsPackageWithVersion =
+        release.getToolsPackageAtVersion();
+    }
+
     try {
       var conn = connectToPackagesStatsServer();
 
@@ -63,7 +81,8 @@ var recordPackages = function () {
 
       conn.call("recordAppPackages",
                 project.project.getAppIdentifier(),
-                packages);
+                packages,
+                userAgentInfo);
     } catch (err) {
       logErrorIfRunningMeteorRelease(err);
       // Do nothing. A failure to record package stats shouldn't be
diff --git a/tools/tests/report-stats.js b/tools/tests/report-stats.js
index fbb9667c6d..d7f3ef5da6 100644
--- a/tools/tests/report-stats.js
+++ b/tools/tests/report-stats.js
@@ -1,5 +1,7 @@
 var _ = require('underscore');
+var os = require("os");
 
+var release = require("../release.js");
 var selftest = require('../selftest.js');
 var testUtils = require('../test-utils.js');
 var stats = require('../stats.js');
@@ -53,6 +55,22 @@ selftest.define("report-stats", ["slow"], function () {
   appPackages = stats.getPackagesForAppIdInTest();
   selftest.expectEqual(appPackages.userId, testUtils.getUserId(s));
 
+  var expectedUserAgentInfo = {
+    hostname: os.hostname(),
+    osPlatform: os.platform(),
+    osType: os.type(),
+    osRelease: os.release(),
+    osArch: os.arch()
+  };
+  if (! release.current.isCheckout()) {
+    expectedUserAgentInfo.meteorReleaseTrack = release.getReleaseTrack();
+    expectedUserAgentInfo.meteorReleaseVersion = release.getReleaseVersion();
+    expectedUserAgentInfo.meteorToolsPackageWithVersion =
+      release.getToolsPackageAtVersion();
+  }
+
+  selftest.expectEqual(appPackages.meta, expectedUserAgentInfo);
+
   // Add the opt-out package, verify that no stats are recorded for the
   // app.
   run = s.run("add", "package-stats-opt-out");

From 4b58ba4d9996dfd68f70951d56e398e794a18b08 Mon Sep 17 00:00:00 2001
From: Emily Stark <emily@meteor.com>
Date: Tue, 24 Jun 2014 19:36:34 -0700
Subject: [PATCH 02/10] A steaming pile of hacks to get report-stats test
 passing.

---
 tools/project.js                              |   5 +-
 tools/stats.js                                |  37 +++-
 .../package-stats-tests/.meteor/.gitignore    |   1 +
 .../package-stats-tests/.meteor/identifier    |   1 +
 .../apps/package-stats-tests/.meteor/packages |   5 +
 .../apps/package-stats-tests/.meteor/release  |   1 +
 .../package-stats-tests.js                    |  10 +
 .../packages/local-package/blah.js            |   1 +
 .../packages/local-package/package.js         |   8 +
 .../packages/package-stats-opt-out/package.js |   9 +
 .../programs/empty-control/attributes.json    |   1 +
 .../programs/empty-control/package.js         |   4 +
 tools/tests/report-stats.js                   | 188 +++++++++++-------
 13 files changed, 195 insertions(+), 76 deletions(-)
 create mode 100644 tools/tests/apps/package-stats-tests/.meteor/.gitignore
 create mode 100644 tools/tests/apps/package-stats-tests/.meteor/identifier
 create mode 100644 tools/tests/apps/package-stats-tests/.meteor/packages
 create mode 100644 tools/tests/apps/package-stats-tests/.meteor/release
 create mode 100644 tools/tests/apps/package-stats-tests/package-stats-tests.js
 create mode 100644 tools/tests/apps/package-stats-tests/packages/local-package/blah.js
 create mode 100644 tools/tests/apps/package-stats-tests/packages/local-package/package.js
 create mode 100644 tools/tests/apps/package-stats-tests/packages/package-stats-opt-out/package.js
 create mode 100644 tools/tests/apps/package-stats-tests/programs/empty-control/attributes.json
 create mode 100644 tools/tests/apps/package-stats-tests/programs/empty-control/package.js

diff --git a/tools/project.js b/tools/project.js
index bbbfa2d3aa..3eccdb6918 100644
--- a/tools/project.js
+++ b/tools/project.js
@@ -330,7 +330,9 @@ _.extend(Project.prototype, {
     return path.join(self.rootDir, '.meteor', 'packages');
   },
 
-  // Give the contents of the project's .meteor/versions file to the caller.
+  // Give the contents of the project's .meteor/versions file to the
+  // caller, possibly after recalculating dependencies and rewriting the
+  // versions file.
   //
   // Returns an object mapping package name to its string version.
   getVersions : function () {
@@ -676,3 +678,4 @@ _.extend(Project.prototype, {
 // cumbersome, that is our general design pattern for singletons (ex:
 // packageCache.packageCache, etc)
 project.project = new Project();
+project.Project = Project;
diff --git a/tools/stats.js b/tools/stats.js
index 56ccef6026..aeb19e6ad4 100644
--- a/tools/stats.js
+++ b/tools/stats.js
@@ -18,11 +18,32 @@ var optOutPackageName = "package-stats-opt-out";
 // indirectly. Formatted as a list of objects with 'name', 'version'
 // and 'direct', which is how the `recordAppPackages` method on the
 // stats server expects to get this list.
-var packageList = function () {
-  var directDeps = project.project.getConstraints();
+//
+// In tests, we want to use the same logic to calculate the list of
+// packages for an app created in a sandbox, but we don't want to run
+// the constraint solver, try to load local packages from the catalog,
+// etc. In particular, we don't want to have to repoint project.project
+// at whatever random app we just created in a sandbox and re-initialize
+// the catalog with its local packages (and then have to undo all that
+// after the test is over). So tests can pass a project.Project as an
+// argument, and we'll calculate the list of packages just by looking at
+// .meteor/packages and .meteor/versions, not by doing anything fancy
+// like running the constraint solver.
+// NOTE: This means that if you pass `_currentProjectForTest`, we assume
+// that it is pointing to a root directory with an existing
+// .meteor/versions file.
+var packageList = function (_currentProjectForTest) {
+  var directDeps = (_currentProjectForTest || project.project).getConstraints();
+
+  var versions;
+  if (_currentProjectForTest) {
+    versions = _currentProjectForTest.dependencies;
+  } else {
+    versions = project.project.getVersions();
+  }
 
   return _.map(
-    project.project.getVersions(),
+    versions,
     function (version, name) {
       return {
         name: name,
@@ -55,10 +76,10 @@ var recordPackages = function () {
     };
 
     if (! release.current.isCheckout()) {
-      userAgentInfo.meteorReleaseTrack = release.getReleaseTrack();
-      userAgentInfo.meteorReleaseVersion = release.getReleaseVersion();
+      userAgentInfo.meteorReleaseTrack = release.current.getReleaseTrack();
+      userAgentInfo.meteorReleaseVersion = release.current.getReleaseVersion();
       userAgentInfo.meteorToolsPackageWithVersion =
-        release.getToolsPackageAtVersion();
+        release.current.getToolsPackageAtVersion();
     }
 
     try {
@@ -102,10 +123,10 @@ var logErrorIfRunningMeteorRelease = function (err) {
 
 // Used in a test (and can only be used against the testing packages
 // server) to fetch one package stats entry for a given application.
-var getPackagesForAppIdInTest = function () {
+var getPackagesForAppIdInTest = function (currentProject) {
   return connectToPackagesStatsServer().call(
     "getPackagesForAppId",
-    project.project.getAppIdentifier());
+    currentProject.getAppIdentifier());
 };
 
 var connectToPackagesStatsServer = function () {
diff --git a/tools/tests/apps/package-stats-tests/.meteor/.gitignore b/tools/tests/apps/package-stats-tests/.meteor/.gitignore
new file mode 100644
index 0000000000..4083037423
--- /dev/null
+++ b/tools/tests/apps/package-stats-tests/.meteor/.gitignore
@@ -0,0 +1 @@
+local
diff --git a/tools/tests/apps/package-stats-tests/.meteor/identifier b/tools/tests/apps/package-stats-tests/.meteor/identifier
new file mode 100644
index 0000000000..357df5354f
--- /dev/null
+++ b/tools/tests/apps/package-stats-tests/.meteor/identifier
@@ -0,0 +1 @@
+l0mpu1sq7jnnxmfesk
\ No newline at end of file
diff --git a/tools/tests/apps/package-stats-tests/.meteor/packages b/tools/tests/apps/package-stats-tests/.meteor/packages
new file mode 100644
index 0000000000..2b350e8d94
--- /dev/null
+++ b/tools/tests/apps/package-stats-tests/.meteor/packages
@@ -0,0 +1,5 @@
+# Meteor packages used by this project, one per line.
+#
+# 'meteor add' and 'meteor remove' will edit this file for you,
+# but you can also edit it by hand.
+local-package
\ No newline at end of file
diff --git a/tools/tests/apps/package-stats-tests/.meteor/release b/tools/tests/apps/package-stats-tests/.meteor/release
new file mode 100644
index 0000000000..621e94f0ec
--- /dev/null
+++ b/tools/tests/apps/package-stats-tests/.meteor/release
@@ -0,0 +1 @@
+none
diff --git a/tools/tests/apps/package-stats-tests/package-stats-tests.js b/tools/tests/apps/package-stats-tests/package-stats-tests.js
new file mode 100644
index 0000000000..c9fcac2859
--- /dev/null
+++ b/tools/tests/apps/package-stats-tests/package-stats-tests.js
@@ -0,0 +1,10 @@
+// This app functions with no packages loaded, so it's good for testing releases
+// with no packages.  All it does is print "RUNNING" and run forever.
+main = function () {
+  // Tell the runner we're up.
+  console.log("LISTENING");
+  // Ensure Node doesn't kill us.
+  process.stdin.resume();
+  // Ensure boot.js doesn't kill us.
+  return 'DAEMON';
+};
diff --git a/tools/tests/apps/package-stats-tests/packages/local-package/blah.js b/tools/tests/apps/package-stats-tests/packages/local-package/blah.js
new file mode 100644
index 0000000000..f2eb4b3e6f
--- /dev/null
+++ b/tools/tests/apps/package-stats-tests/packages/local-package/blah.js
@@ -0,0 +1 @@
+console.log("BLAH");
diff --git a/tools/tests/apps/package-stats-tests/packages/local-package/package.js b/tools/tests/apps/package-stats-tests/packages/local-package/package.js
new file mode 100644
index 0000000000..83a22bb181
--- /dev/null
+++ b/tools/tests/apps/package-stats-tests/packages/local-package/package.js
@@ -0,0 +1,8 @@
+Package.describe({
+  summary: "a package",
+  version: "1.0.0"
+});
+
+Package.on_use(function (api) {
+  api.add_files("blah.js");
+});
diff --git a/tools/tests/apps/package-stats-tests/packages/package-stats-opt-out/package.js b/tools/tests/apps/package-stats-tests/packages/package-stats-opt-out/package.js
new file mode 100644
index 0000000000..3dfc770eb1
--- /dev/null
+++ b/tools/tests/apps/package-stats-tests/packages/package-stats-opt-out/package.js
@@ -0,0 +1,9 @@
+// This is a replica of the core package-stats-opt-out package. It
+// exists to make it so that we can add package-stats-opt-out to this
+// app without needing to be using a release that has core packages in
+// it (such as using a sandbox created with a `warehouse` argument).
+
+Package.describe({
+  summary: "a replica of a core package",
+  version: "1.0.0"
+});
diff --git a/tools/tests/apps/package-stats-tests/programs/empty-control/attributes.json b/tools/tests/apps/package-stats-tests/programs/empty-control/attributes.json
new file mode 100644
index 0000000000..11108c5002
--- /dev/null
+++ b/tools/tests/apps/package-stats-tests/programs/empty-control/attributes.json
@@ -0,0 +1 @@
+{"isControlProgram": true}
diff --git a/tools/tests/apps/package-stats-tests/programs/empty-control/package.js b/tools/tests/apps/package-stats-tests/programs/empty-control/package.js
new file mode 100644
index 0000000000..ceb498734a
--- /dev/null
+++ b/tools/tests/apps/package-stats-tests/programs/empty-control/package.js
@@ -0,0 +1,4 @@
+// Empty program. Means we don't need a ctl package.
+Package.describe({
+  version: "1.0.0"
+});
diff --git a/tools/tests/report-stats.js b/tools/tests/report-stats.js
index d7f3ef5da6..4230897ebf 100644
--- a/tools/tests/report-stats.js
+++ b/tools/tests/report-stats.js
@@ -15,93 +15,147 @@ process.env.METEOR_PACKAGE_STATS_SERVER_URL = testStatsServer;
 // than 30 minutes. This is because the `fetchAppPackageUsage` method
 // works by passing an hour time range.
 selftest.define("report-stats", ["slow"], function () {
-  var s = new Sandbox;
+  _.each(
+    // If we are currently running from a checkout, then we run this
+    // test twice (once in which the sandbox uses the current checkout,
+    // and another in which the sandbox uses a simulated release). If we
+    // are currently running from a release, then we can only have the
+    // sandbox use a simulated release that is the same as our current
+    // release (we can't simulate a checkout).
+    release.current.isCheckout() ? [true, false] : [false],
+    function (useFakeRelease) {
+      var sandboxOpts;
+      if (useFakeRelease) {
+        sandboxOpts = {
+          warehouse: {
+            v1: { recommended: true }
+          }
+        };
+      }
+      var s = new Sandbox(sandboxOpts);
+      var run;
 
-  var run = s.run("create", "foo");
-  run.expectExit(0);
-  s.cd("foo");
+      if (useFakeRelease) {
+        // This makes packages not depend on meteor (specifically, makes
+        // our empty control program not depend on meteor). We don't
+        // want to depend on meteor when running from a fake release,
+        // because fake releases don't have any packages.
+        s.set("NO_METEOR_PACKAGE", "t");
+      }
 
-  project.project.setRootDir(s.cwd);
+      s.createApp("foo", "package-stats-tests");
+      s.cd("foo");
+      if (useFakeRelease) {
+        s.write('.meteor/release', 'METEOR-CORE@v1');
+      }
 
-  // verify that identifier file exists for new apps
-  var identifier = s.read(".meteor/identifier");
-  selftest.expectEqual(!! identifier, true);
-  selftest.expectEqual(identifier.length > 0, true);
+      var sandboxProject = new project.Project();
+      sandboxProject.setRootDir(s.cwd);
 
-  // verify that identifier file when running 'meteor bundle' on apps
-  // with no identifier file (eg pre-0.9.0 apps)
-  bundleWithFreshIdentifier(s);
-  identifier = s.read(".meteor/identifier");
-  selftest.expectEqual(!! identifier, true);
-  selftest.expectEqual(identifier.length > 0, true);
+      // XXX test that local-package is a direct dep
 
-  // we just ran 'meteor bundle' so let's test that we actually sent
-  // package usage stats
-  var usage = fetchPackageUsageForApp(identifier);
-  selftest.expectEqual(_.sortBy(usage.packages, "name"),
-                       _.sortBy(stats.packageList(), "name"));
+      // verify that identifier file exists for new apps
+      var identifier = s.read(".meteor/identifier");
+      selftest.expectEqual(!! identifier, true);
+      selftest.expectEqual(identifier.length > 0, true);
 
-  // verify that the stats server recorded that with no userId
-  var appPackages = stats.getPackagesForAppIdInTest();
-  selftest.expectEqual(appPackages.appId, identifier);
-  selftest.expectEqual(appPackages.userId, null);
-  selftest.expectEqual(_.sortBy(appPackages.packages, "name"),
-                       _.sortBy(stats.packageList(), "name"));
+      // verify that identifier file when running 'meteor bundle' on apps
+      // with no identifier file (eg pre-0.9.0 apps)
+      bundleWithFreshIdentifier(s, sandboxProject);
 
-  // now bundle again while logged in. verify that the stats server
-  // recorded that with the right userId
-  testUtils.login(s, "test", "testtest");
-  bundleWithFreshIdentifier(s);
-  appPackages = stats.getPackagesForAppIdInTest();
-  selftest.expectEqual(appPackages.userId, testUtils.getUserId(s));
+      identifier = s.read(".meteor/identifier");
+      selftest.expectEqual(!! identifier, true);
+      selftest.expectEqual(identifier.length > 0, true);
 
-  var expectedUserAgentInfo = {
-    hostname: os.hostname(),
-    osPlatform: os.platform(),
-    osType: os.type(),
-    osRelease: os.release(),
-    osArch: os.arch()
-  };
-  if (! release.current.isCheckout()) {
-    expectedUserAgentInfo.meteorReleaseTrack = release.getReleaseTrack();
-    expectedUserAgentInfo.meteorReleaseVersion = release.getReleaseVersion();
-    expectedUserAgentInfo.meteorToolsPackageWithVersion =
-      release.getToolsPackageAtVersion();
-  }
+      // we just ran 'meteor bundle' so let's test that we actually sent
+      // package usage stats
+      var usage = fetchPackageUsageForApp(identifier);
+      selftest.expectEqual(_.sortBy(usage.packages, "name"),
+                           _.sortBy(stats.packageList(sandboxProject), "name"));
 
-  selftest.expectEqual(appPackages.meta, expectedUserAgentInfo);
+      // verify that the stats server recorded that with no userId
+      var appPackages = stats.getPackagesForAppIdInTest(sandboxProject);
+      selftest.expectEqual(appPackages.appId, identifier);
+      selftest.expectEqual(appPackages.userId, null);
+      selftest.expectEqual(_.sortBy(appPackages.packages, "name"),
+                           _.sortBy(stats.packageList(sandboxProject), "name"));
 
-  // Add the opt-out package, verify that no stats are recorded for the
-  // app.
-  run = s.run("add", "package-stats-opt-out");
-  run.waitSecs(15);
-  run.expectExit(0);
-  bundleWithFreshIdentifier(s);
-  appPackages = stats.getPackagesForAppIdInTest();
-  selftest.expectEqual(appPackages, undefined);
+      // now bundle again while logged in. verify that the stats server
+      // recorded that with the right userId
+      testUtils.login(s, "test", "testtest");
+      bundleWithFreshIdentifier(s, sandboxProject);
+      appPackages = stats.getPackagesForAppIdInTest(sandboxProject);
+      selftest.expectEqual(appPackages.userId, testUtils.getUserId(s));
 
-  // Remove the opt-out package, verify that stats get sent again.
-  run = s.run("remove", "package-stats-opt-out");
-  run.waitSecs(15);
-  run.expectExit(0);
-  bundle(s);
-  appPackages = stats.getPackagesForAppIdInTest();
-  selftest.expectEqual(appPackages.userId, testUtils.getUserId(s));
-  selftest.expectEqual(_.sortBy(appPackages.packages, "name"),
-                       _.sortBy(stats.packageList(), "name"));
+      var expectedUserAgentInfo = {
+        hostname: os.hostname(),
+        osPlatform: os.platform(),
+        osType: os.type(),
+        osRelease: os.release(),
+        osArch: os.arch()
+      };
+      if (useFakeRelease) {
+        expectedUserAgentInfo.meteorReleaseTrack =
+          "METEOR-CORE";
+        expectedUserAgentInfo.meteorReleaseVersion =
+          "v1";
+
+        // Check the tools package version against a regexp and then
+        // delete it from `appPackages.meta` so that we can check the
+        // rest of `appPackages.meta` with `expectEqual`.
+        var toolsPackageWithVersion =
+              appPackages.meta.meteorToolsPackageWithVersion;
+        if (! toolsPackageWithVersion.match(
+              /meteor-tool@\d.\d.\d(\+[a-z0-9]+)?/)) {
+          selftest.fail();
+        }
+        delete appPackages.meta.meteorToolsPackageWithVersion;
+      }
+
+      selftest.expectEqual(appPackages.meta, expectedUserAgentInfo);
+
+      // Add the opt-out package, verify that no stats are recorded for the
+      // app.
+      //
+      // XXX The app has a local package-stats-opt-out package in it, so
+      // that we can add the opt-out package without needing to be using
+      // a release that knows about the opt-out package. (Our sandbox
+      // release only has the tool package and no others.) In the near
+      // future we should just have a way to simulate a release in a
+      // sandbox that knows about all the packages in the meteor release
+      // or checkout that is running 'meteor self-test'. That will
+      // simplify this test a lot.
+      run = s.run("add", "package-stats-opt-out");
+      run.waitSecs(15);
+      run.expectExit(0);
+      bundleWithFreshIdentifier(s, sandboxProject);
+      appPackages = stats.getPackagesForAppIdInTest(sandboxProject);
+      selftest.expectEqual(appPackages, undefined);
+
+      // Remove the opt-out package, verify that stats get sent again.
+      run = s.run("remove", "package-stats-opt-out");
+      run.waitSecs(15);
+      run.expectExit(0);
+      bundle(s, sandboxProject);
+      appPackages = stats.getPackagesForAppIdInTest(sandboxProject);
+      selftest.expectEqual(appPackages.userId, testUtils.getUserId(s));
+      selftest.expectEqual(_.sortBy(appPackages.packages, "name"),
+                           _.sortBy(stats.packageList(sandboxProject), "name"));
+    }
+  );
 });
 
 // Bundle the app in the current working directory after deleting its
 // identifier file (meaning a new one will be created).
 // @param s {Sandbox}
-var bundleWithFreshIdentifier = function (s) {
+var bundleWithFreshIdentifier = function (s, sandboxProject) {
   s.unlink(".meteor/identifier");
-  bundle(s);
+  bundle(s, sandboxProject);
 };
 
 // Bundle the app in the current working directory.
 // @param s {Sandbox}
-var bundle = function (s) {
+var bundle = function (s, sandboxProject) {
   var run = s.run("bundle", "foo.tar.gz");
   run.waitSecs(30);
   run.expectExit(0);
@@ -109,7 +163,7 @@ var bundle = function (s) {
   // XXX not sure why this is necessary (i.e. why project can't detect
   // that .meteor/identifier or .meteor/packages has changed and figure
   // out that it needs to reload itself)
-  project.project.reload();
+  sandboxProject.reload();
 };
 
 // Contact the package stats server and look for a given app

From e1d4140f35b7d411dc22369605ece5b92f24ac8a Mon Sep 17 00:00:00 2001
From: Emily Stark <emily@meteor.com>
Date: Tue, 24 Jun 2014 19:45:12 -0700
Subject: [PATCH 03/10] Add a selftest.fail() reason

---
 tools/tests/report-stats.js | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/tests/report-stats.js b/tools/tests/report-stats.js
index 4230897ebf..e70aacd193 100644
--- a/tools/tests/report-stats.js
+++ b/tools/tests/report-stats.js
@@ -107,7 +107,8 @@ selftest.define("report-stats", ["slow"], function () {
               appPackages.meta.meteorToolsPackageWithVersion;
         if (! toolsPackageWithVersion.match(
               /meteor-tool@\d.\d.\d(\+[a-z0-9]+)?/)) {
-          selftest.fail();
+          selftest.fail("Tools package with version is " +
+                        "not correct: " + toolsPackageWithVersion);
         }
         delete appPackages.meta.meteorToolsPackageWithVersion;
       }

From e4ecda9fe8d2fd08afb4f014bf998ba3ba37340a Mon Sep 17 00:00:00 2001
From: Emily Stark <emily@meteor.com>
Date: Tue, 24 Jun 2014 19:45:25 -0700
Subject: [PATCH 04/10] Fix direct dependency reporting.

And test for it.
---
 tools/stats.js              | 2 +-
 tools/tests/report-stats.js | 8 ++++++++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/stats.js b/tools/stats.js
index aeb19e6ad4..c0626ca303 100644
--- a/tools/stats.js
+++ b/tools/stats.js
@@ -48,7 +48,7 @@ var packageList = function (_currentProjectForTest) {
       return {
         name: name,
         version: version,
-        direct: _.contains(directDeps, name)
+        direct: _.has(directDeps, name)
       };
     }
   );
diff --git a/tools/tests/report-stats.js b/tools/tests/report-stats.js
index e70aacd193..0edc6222d6 100644
--- a/tools/tests/report-stats.js
+++ b/tools/tests/report-stats.js
@@ -73,6 +73,14 @@ selftest.define("report-stats", ["slow"], function () {
       selftest.expectEqual(_.sortBy(usage.packages, "name"),
                            _.sortBy(stats.packageList(sandboxProject), "name"));
 
+      // Check that the direct dependency was recorded as such.
+      _.each(usage.packages, function (package) {
+        if (package.name === "local-package" &&
+            ! package.direct) {
+          selftest.fail("local-package is not marked as a direct dependency");
+        }
+      });
+
       // verify that the stats server recorded that with no userId
       var appPackages = stats.getPackagesForAppIdInTest(sandboxProject);
       selftest.expectEqual(appPackages.appId, identifier);

From f761e898c84d58c2cfe8bc0aeb547d6386d39799 Mon Sep 17 00:00:00 2001
From: Emily Stark <emily@meteor.com>
Date: Tue, 24 Jun 2014 20:23:41 -0700
Subject: [PATCH 05/10] Record session id on package stats

---
 tools/auth.js               |  5 ++-
 tools/run-app.js            |  2 +-
 tools/stats.js              |  3 +-
 tools/tests/report-stats.js | 76 ++++++++++++++++++++++++-------------
 4 files changed, 55 insertions(+), 31 deletions(-)

diff --git a/tools/auth.js b/tools/auth.js
index 7f43d901fa..2fae2bb5fe 100644
--- a/tools/auth.js
+++ b/tools/auth.js
@@ -1009,8 +1009,9 @@ session.registrationUrl + "\n\n");
 
 exports.tryRevokeOldTokens = tryRevokeOldTokens;
 
-exports.getSessionId = function (domain) {
-  return getSession(readSessionData(), domain).session;
+exports.getSessionId = function (domain, sessionData) {
+  sessionData = sessionData || readSessionData();
+  return getSession(sessionData, domain).session;
 };
 
 exports.setSessionId = function (domain, sessionId) {
diff --git a/tools/run-app.js b/tools/run-app.js
index 0521702172..7c37da1642 100644
--- a/tools/run-app.js
+++ b/tools/run-app.js
@@ -275,7 +275,7 @@ _.extend(AppProcess.prototype, {
 //
 // - Other options: appDirForVersionCheck (defaults to appDir), port,
 //   mongoUrl, oplogUrl, buildOptions, rootUrl, settingsFile, program,
-//   proxy, dontRecordPackageUsage
+//   proxy, recordPackageUsage
 //
 // To use, construct an instance of AppRunner, and then call start() to start it
 // running. To stop it, either return false from onRunEnd, or call stop().  (But
diff --git a/tools/stats.js b/tools/stats.js
index c0626ca303..45712e3e4e 100644
--- a/tools/stats.js
+++ b/tools/stats.js
@@ -72,7 +72,8 @@ var recordPackages = function () {
       osPlatform: os.platform(),
       osType: os.type(),
       osRelease: os.release(),
-      osArch: os.arch()
+      osArch: os.arch(),
+      sessionId: auth.getSessionId(config.getAccountsDomain())
     };
 
     if (! release.current.isCheckout()) {
diff --git a/tools/tests/report-stats.js b/tools/tests/report-stats.js
index 0edc6222d6..9f3be405de 100644
--- a/tools/tests/report-stats.js
+++ b/tools/tests/report-stats.js
@@ -1,6 +1,8 @@
 var _ = require('underscore');
 var os = require("os");
 
+var auth = require("../auth.js");
+var config = require("../config.js");
 var release = require("../release.js");
 var selftest = require('../selftest.js');
 var testUtils = require('../test-utils.js');
@@ -11,6 +13,38 @@ var project = require('../project.js');
 var testStatsServer = "https://test-package-stats.meteor.com";
 process.env.METEOR_PACKAGE_STATS_SERVER_URL = testStatsServer;
 
+var checkMeta = function (appPackages, sessionId, useFakeRelease) {
+  var expectedUserAgentInfo = {
+    hostname: os.hostname(),
+    osPlatform: os.platform(),
+    osType: os.type(),
+    osRelease: os.release(),
+    osArch: os.arch(),
+    sessionId: sessionId
+  };
+  if (useFakeRelease) {
+    expectedUserAgentInfo.meteorReleaseTrack =
+      "METEOR-CORE";
+    expectedUserAgentInfo.meteorReleaseVersion =
+      "v1";
+
+    // Check the tools package version against a regexp and then
+    // delete it from `appPackages.meta` so that we can check the
+    // rest of `appPackages.meta` with `expectEqual`.
+    var toolsPackageWithVersion =
+          appPackages.meta.meteorToolsPackageWithVersion;
+    if (! toolsPackageWithVersion.match(
+        /meteor-tool@\d.\d.\d(\+[a-z0-9]+)?/)) {
+      selftest.fail("Tools package with version is " +
+                    "not correct: " + toolsPackageWithVersion);
+    }
+    delete appPackages.meta.meteorToolsPackageWithVersion;
+  }
+
+  selftest.expectEqual(appPackages.meta, expectedUserAgentInfo);
+
+};
+
 // NOTE: This test will fail if your machine's time is skewed by more
 // than 30 minutes. This is because the `fetchAppPackageUsage` method
 // works by passing an hour time range.
@@ -52,6 +86,8 @@ selftest.define("report-stats", ["slow"], function () {
       var sandboxProject = new project.Project();
       sandboxProject.setRootDir(s.cwd);
 
+      var sessionId;
+
       // XXX test that local-package is a direct dep
 
       // verify that identifier file exists for new apps
@@ -87,41 +123,27 @@ selftest.define("report-stats", ["slow"], function () {
       selftest.expectEqual(appPackages.userId, null);
       selftest.expectEqual(_.sortBy(appPackages.packages, "name"),
                            _.sortBy(stats.packageList(sandboxProject), "name"));
+      checkMeta(appPackages, sessionId, useFakeRelease);
 
       // now bundle again while logged in. verify that the stats server
-      // recorded that with the right userId
+      // recorded that with the right userId and meta information
       testUtils.login(s, "test", "testtest");
+      sessionId = auth.getSessionId(config.getAccountsDomain(),
+                                    JSON.parse(s.readSessionFile()));
+
       bundleWithFreshIdentifier(s, sandboxProject);
       appPackages = stats.getPackagesForAppIdInTest(sandboxProject);
       selftest.expectEqual(appPackages.userId, testUtils.getUserId(s));
+      checkMeta(appPackages, sessionId, useFakeRelease);
 
-      var expectedUserAgentInfo = {
-        hostname: os.hostname(),
-        osPlatform: os.platform(),
-        osType: os.type(),
-        osRelease: os.release(),
-        osArch: os.arch()
-      };
-      if (useFakeRelease) {
-        expectedUserAgentInfo.meteorReleaseTrack =
-          "METEOR-CORE";
-        expectedUserAgentInfo.meteorReleaseVersion =
-          "v1";
+      // Log out, and then test that our session id still gets recorded.
+      testUtils.logout(s);
+      bundleWithFreshIdentifier(s, sandboxProject);
+      appPackages = stats.getPackagesForAppIdInTest(sandboxProject);
+      selftest.expectEqual(appPackages.userId, null);
+      checkMeta(appPackages, sessionId, useFakeRelease);
 
-        // Check the tools package version against a regexp and then
-        // delete it from `appPackages.meta` so that we can check the
-        // rest of `appPackages.meta` with `expectEqual`.
-        var toolsPackageWithVersion =
-              appPackages.meta.meteorToolsPackageWithVersion;
-        if (! toolsPackageWithVersion.match(
-              /meteor-tool@\d.\d.\d(\+[a-z0-9]+)?/)) {
-          selftest.fail("Tools package with version is " +
-                        "not correct: " + toolsPackageWithVersion);
-        }
-        delete appPackages.meta.meteorToolsPackageWithVersion;
-      }
-
-      selftest.expectEqual(appPackages.meta, expectedUserAgentInfo);
+      testUtils.login(s, "test", "testtest");
 
       // Add the opt-out package, verify that no stats are recorded for the
       // app.

From e525cb51bb31e1b3c1b2ce89f3a70584403a4f7b Mon Sep 17 00:00:00 2001
From: Emily Stark <emily@meteor.com>
Date: Tue, 24 Jun 2014 20:23:49 -0700
Subject: [PATCH 06/10] Fix bug in `Run.stop()`.

Otherwise selftest crashes if you call `run.stop()` without calling
e.g. `run.match()` first.
---
 tools/selftest.js | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/selftest.js b/tools/selftest.js
index 4c18c89a72..4950a3f9e4 100644
--- a/tools/selftest.js
+++ b/tools/selftest.js
@@ -803,6 +803,7 @@ _.extend(Run.prototype, {
   stop: markStack(function () {
     var self = this;
     if (self.exitStatus === undefined) {
+      self._ensureStarted();
       self.proc.kill();
       self.expectExit();
     }

From 01b73cafce9894c7ae08b42193f8034b8c3b189d Mon Sep 17 00:00:00 2001
From: Emily Stark <emily@meteor.com>
Date: Tue, 24 Jun 2014 20:25:19 -0700
Subject: [PATCH 07/10] Remove obsolete XXX

---
 tools/tests/report-stats.js | 2 --
 1 file changed, 2 deletions(-)

diff --git a/tools/tests/report-stats.js b/tools/tests/report-stats.js
index 9f3be405de..17d1d38bee 100644
--- a/tools/tests/report-stats.js
+++ b/tools/tests/report-stats.js
@@ -88,8 +88,6 @@ selftest.define("report-stats", ["slow"], function () {
 
       var sessionId;
 
-      // XXX test that local-package is a direct dep
-
       // verify that identifier file exists for new apps
       var identifier = s.read(".meteor/identifier");
       selftest.expectEqual(!! identifier, true);

From e8b1fddc4e5cd1130ef7f6514c655768cbed4b0f Mon Sep 17 00:00:00 2001
From: Emily Stark <emily@meteor.com>
Date: Wed, 25 Jun 2014 09:52:47 -0700
Subject: [PATCH 08/10] Do a 'meteor run' in report-stats test.

It tests that stats get sent on 'meteor run', and also is faster than
bundling.
---
 tools/tests/apps/package-stats-tests/.meteor/identifier | 1 -
 tools/tests/report-stats.js                             | 5 ++++-
 2 files changed, 4 insertions(+), 2 deletions(-)
 delete mode 100644 tools/tests/apps/package-stats-tests/.meteor/identifier

diff --git a/tools/tests/apps/package-stats-tests/.meteor/identifier b/tools/tests/apps/package-stats-tests/.meteor/identifier
deleted file mode 100644
index 357df5354f..0000000000
--- a/tools/tests/apps/package-stats-tests/.meteor/identifier
+++ /dev/null
@@ -1 +0,0 @@
-l0mpu1sq7jnnxmfesk
\ No newline at end of file
diff --git a/tools/tests/report-stats.js b/tools/tests/report-stats.js
index 17d1d38bee..49511787f6 100644
--- a/tools/tests/report-stats.js
+++ b/tools/tests/report-stats.js
@@ -136,10 +136,13 @@ selftest.define("report-stats", ["slow"], function () {
 
       // Log out, and then test that our session id still gets recorded.
       testUtils.logout(s);
-      bundleWithFreshIdentifier(s, sandboxProject);
+      run = s.run("run");
+      run.waitSecs(5);
+      run.match("BLAH");
       appPackages = stats.getPackagesForAppIdInTest(sandboxProject);
       selftest.expectEqual(appPackages.userId, null);
       checkMeta(appPackages, sessionId, useFakeRelease);
+      run.stop();
 
       testUtils.login(s, "test", "testtest");
 

From 1a3fd674969cefa73b1c77e10d677f5c18aeae9a Mon Sep 17 00:00:00 2001
From: Emily Stark <emily@meteor.com>
Date: Wed, 25 Jun 2014 10:33:14 -0700
Subject: [PATCH 09/10] Add 'clientAddress' to package stats meta test.

Currently fails because 'meteor deploy' sites don't have
HTTP_FORWARDED_COUNT set. I think the right way to fix this is to have
the runner set it.
---
 tools/tests/report-stats.js | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/tools/tests/report-stats.js b/tools/tests/report-stats.js
index 49511787f6..109534341b 100644
--- a/tools/tests/report-stats.js
+++ b/tools/tests/report-stats.js
@@ -13,14 +13,21 @@ var project = require('../project.js');
 var testStatsServer = "https://test-package-stats.meteor.com";
 process.env.METEOR_PACKAGE_STATS_SERVER_URL = testStatsServer;
 
+var clientAddress;
+
 var checkMeta = function (appPackages, sessionId, useFakeRelease) {
+  if (! clientAddress) {
+    clientAddress = getClientAddress();
+  }
+
   var expectedUserAgentInfo = {
     hostname: os.hostname(),
     osPlatform: os.platform(),
     osType: os.type(),
     osRelease: os.release(),
     osArch: os.arch(),
-    sessionId: sessionId
+    sessionId: sessionId,
+    clientAddress: clientAddress
   };
   if (useFakeRelease) {
     expectedUserAgentInfo.meteorReleaseTrack =
@@ -229,3 +236,8 @@ var fetchPackageUsageForApp = function (identifier) {
 
   return found;
 };
+
+var getClientAddress = function () {
+  var stats = testUtils.ddpConnect(testStatsServer);
+  return stats.call("getClientAddress");
+};

From 3036b6b686703ed80b7af228621a69d3aab56f7c Mon Sep 17 00:00:00 2001
From: Emily Stark <emily@meteor.com>
Date: Wed, 25 Jun 2014 10:51:06 -0700
Subject: [PATCH 10/10] Use `EJSON.equals` in `selftest.expectEqual`

---
 tools/selftest.js           | 6 ++++--
 tools/tests/report-stats.js | 6 +++++-
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/tools/selftest.js b/tools/selftest.js
index 4950a3f9e4..8bfbf622ca 100644
--- a/tools/selftest.js
+++ b/tools/selftest.js
@@ -9,6 +9,9 @@ var catalog = require('./catalog.js');
 var archinfo = require('./archinfo.js');
 var packageLoader = require('./package-loader.js');
 var Future = require('fibers/future');
+var uniload = require('./uniload.js');
+
+var Package = uniload.load({ packages: ["ejson"] });
 
 // Exception representing a test failure
 var TestFailure = function (reason, details) {
@@ -34,8 +37,7 @@ var fail = markStack(function (reason) {
 // with 'actual' being the value that the test got and 'expected'
 // being the expected value
 var expectEqual = markStack(function (actual, expected) {
-  // XXX Super janky. Should use EJSON.equals for a deep equality test.
-  if (JSON.stringify(actual) !== JSON.stringify(expected)) {
+  if (! Package.ejson.EJSON.equals(actual, expected)) {
     throw new TestFailure("not-equal", {
       expected: expected,
       actual: actual
diff --git a/tools/tests/report-stats.js b/tools/tests/report-stats.js
index 109534341b..aa6b5bcf71 100644
--- a/tools/tests/report-stats.js
+++ b/tools/tests/report-stats.js
@@ -26,9 +26,13 @@ var checkMeta = function (appPackages, sessionId, useFakeRelease) {
     osType: os.type(),
     osRelease: os.release(),
     osArch: os.arch(),
-    sessionId: sessionId,
     clientAddress: clientAddress
   };
+
+  if (sessionId) {
+    expectedUserAgentInfo.sessionId = sessionId;
+  }
+
   if (useFakeRelease) {
     expectedUserAgentInfo.meteorReleaseTrack =
       "METEOR-CORE";