You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cordova.apache.org by ra...@apache.org on 2019/11/01 21:25:50 UTC
[cordova-cli] branch master updated: Fix blocked telemetry calls
(#473)
This is an automated email from the ASF dual-hosted git repository.
raphinesse pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cordova-cli.git
The following commit(s) were added to refs/heads/master by this push:
new 3d4528e Fix blocked telemetry calls (#473)
3d4528e is described below
commit 3d4528e852423835400f6f0f5166eb2589ffd51e
Author: Raphael von der Grün <ra...@gmail.com>
AuthorDate: Fri Nov 1 22:25:40 2019 +0100
Fix blocked telemetry calls (#473)
* Check that we're actually sending telemetry data
This uncovers three cases where we don't
* fix: unmute certain telemetry.track calls
---
spec/cli.spec.js | 37 ++++++++++++++++++++++++++++---------
spec/telemetry.spec.js | 20 ++++++++++++++------
src/telemetry.js | 17 ++++++++++++++---
3 files changed, 56 insertions(+), 18 deletions(-)
diff --git a/spec/cli.spec.js b/spec/cli.spec.js
index 2ee1167..51bbb09 100644
--- a/spec/cli.spec.js
+++ b/spec/cli.spec.js
@@ -243,15 +243,28 @@ describe('cordova cli', () => {
});
describe('telemetry', () => {
+ const Insight = require('insight');
+ let isOptedOut;
+
beforeEach(() => {
+ // Allow testing if we _really_ would send tracking requests
+ telemetry.track.and.callThrough();
+ telemetry.turnOn.and.callThrough();
+ telemetry.turnOff.and.callThrough();
+ spyOn(Insight.prototype, 'track').and.callThrough();
+ spyOn(Insight.prototype, '_save');
+ spyOnProperty(Insight.prototype, 'optOut', 'get')
+ .and.callFake(() => isOptedOut);
+ spyOnProperty(Insight.prototype, 'optOut', 'set')
+ .and.callFake(x => { isOptedOut = x; });
+
// Set a normal opted-in user as default
spyOn(telemetry, 'isCI').and.returnValue(false);
- spyOn(telemetry, 'isOptedIn').and.returnValue(true);
- spyOn(telemetry, 'hasUserOptedInOrOut').and.returnValue(true);
+ isOptedOut = false;
});
it("Test#023 : skips prompt when user runs 'cordova telemetry X'", () => {
- telemetry.hasUserOptedInOrOut.and.returnValue(false);
+ isOptedOut = undefined;
return Promise.resolve()
.then(_ => cli(['node', 'cordova', 'telemetry', 'on']))
@@ -262,16 +275,18 @@ describe('cordova cli', () => {
});
it("Test#024 : is NOT collected when user runs 'cordova telemetry on' while NOT opted-in", () => {
- telemetry.isOptedIn.and.returnValue(false);
+ isOptedOut = true;
return cli(['node', 'cordova', 'telemetry', 'on']).then(() => {
- expect(telemetry.track).not.toHaveBeenCalled();
+ expect(Insight.prototype.track).not.toHaveBeenCalled();
});
});
it("Test#025 : is collected when user runs 'cordova telemetry off' while opted-in", () => {
return cli(['node', 'cordova', 'telemetry', 'off']).then(() => {
expect(telemetry.track).toHaveBeenCalledWith('telemetry', 'off', 'via-cordova-telemetry-cmd', 'successful');
+ expect(Insight.prototype.track).toHaveBeenCalled();
+ expect(Insight.prototype._save).toHaveBeenCalled();
});
});
@@ -280,12 +295,14 @@ describe('cordova cli', () => {
return cli(['node', 'cordova', 'platform', 'add', 'ios']).then(() => {
expect(telemetry.track).toHaveBeenCalledWith('platform', 'add', 'successful');
+ expect(Insight.prototype.track).toHaveBeenCalled();
+ expect(Insight.prototype._save).toHaveBeenCalled();
});
});
it('Test#027 : shows prompt if user neither opted in or out yet', () => {
+ isOptedOut = undefined;
spyOn(cordova, 'prepare').and.returnValue(Promise.resolve());
- telemetry.hasUserOptedInOrOut.and.returnValue(false);
return cli(['node', 'cordova', 'prepare']).then(() => {
expect(telemetry.showPrompt).toHaveBeenCalled();
@@ -301,20 +318,22 @@ describe('cordova cli', () => {
});
it("Test#030 : is NOT collected when --no-telemetry flag found and doesn't prompt", () => {
- telemetry.hasUserOptedInOrOut.and.returnValue(false);
+ isOptedOut = undefined;
return cli(['node', 'cordova', '--version', '--no-telemetry']).then(() => {
expect(telemetry.showPrompt).not.toHaveBeenCalled();
- expect(telemetry.track).not.toHaveBeenCalled();
+ expect(Insight.prototype.track).not.toHaveBeenCalled();
});
});
it("Test#033 : track opt-out that happened via 'cordova telemetry off' even if user is NOT opted-in ", () => {
- telemetry.isOptedIn.and.returnValue(false);
+ isOptedOut = true;
return cli(['node', 'cordova', 'telemetry', 'off']).then(() => {
expect(telemetry.isOptedIn()).toBeFalsy();
expect(telemetry.track).toHaveBeenCalledWith('telemetry', 'off', 'via-cordova-telemetry-cmd', 'successful');
+ expect(Insight.prototype.track).toHaveBeenCalled();
+ expect(Insight.prototype._save).toHaveBeenCalled();
});
});
});
diff --git a/spec/telemetry.spec.js b/spec/telemetry.spec.js
index ed0360f..147b736 100644
--- a/spec/telemetry.spec.js
+++ b/spec/telemetry.spec.js
@@ -39,9 +39,12 @@ describe('telemetry', () => {
// Prevent any settings from being persisted during testing
insight.config = {
- get: jasmine.createSpy('insight.config.get'),
- set: jasmine.createSpy('insight.config.set')
+ get (key) { return this[key]; },
+ set (key, val) { this[key] = val; }
};
+ for (const key in insight.config) {
+ spyOn(insight.config, key).and.callThrough();
+ }
// Prevent tracking anything during testing
spyOn(Insight.prototype, '_save');
@@ -134,9 +137,12 @@ describe('telemetry', () => {
beforeEach(() => {
spyOn(console, 'log');
- spyOn(telemetry, 'track');
+ spyOn(telemetry, 'track').and.callThrough();
response = Symbol('response');
- insight.askPermission.and.callFake((_, cb) => cb(null, response));
+ insight.askPermission.and.callFake((_, cb) => {
+ insight.optOut = !response;
+ cb(null, response);
+ });
});
it('calls insight.askPermission [T011]', () => {
@@ -169,6 +175,7 @@ describe('telemetry', () => {
expect(telemetry.track).toHaveBeenCalledWith(
'telemetry', 'on', 'via-cli-prompt-choice', 'successful'
);
+ expect(Insight.prototype._save).toHaveBeenCalled();
});
});
});
@@ -197,6 +204,7 @@ describe('telemetry', () => {
expect(telemetry.track).toHaveBeenCalledWith(
'telemetry', 'off', 'via-cli-prompt-choice', 'successful'
);
+ expect(Insight.prototype._save).toHaveBeenCalled();
});
});
});
@@ -268,10 +276,10 @@ describe('telemetry', () => {
expect(insight._save).toHaveBeenCalled();
});
- it('does NOT track when user opted out [T023]', () => {
+ it('still tracks when user opted out [T023]', () => {
insight.config.get.and.returnValue(true);
insight.track();
- expect(insight._save).not.toHaveBeenCalled();
+ expect(insight._save).toHaveBeenCalled();
});
describe('on CI', () => {
diff --git a/src/telemetry.js b/src/telemetry.js
index 55765f7..f307e67 100644
--- a/src/telemetry.js
+++ b/src/telemetry.js
@@ -23,7 +23,18 @@ var GA_TRACKING_CODE = 'UA-64283057-7';
var pkg = require('../package.json');
var Insight = require('insight');
-var insight = new Insight({
+
+/**
+ * By redefining `get optOut` we trick Insight into tracking
+ * even though the user might have opted out.
+ */
+class RelentlessInsight extends Insight {
+ get optOut () { return false; }
+ set optOut (value) { super.optOut = value; }
+ get realOptOut () { return super.optOut; }
+}
+
+var insight = new RelentlessInsight({
trackingCode: GA_TRACKING_CODE,
pkg: pkg
});
@@ -74,14 +85,14 @@ function clear () {
}
function isOptedIn () {
- return !insight.optOut;
+ return !insight.realOptOut;
}
/**
* Has the user already answered the telemetry prompt? (thereby opting in or out?)
*/
function hasUserOptedInOrOut () {
- var insightOptOut = insight.optOut === undefined;
+ var insightOptOut = insight.realOptOut === undefined;
return !(insightOptOut);
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org