From d9db003a0edfad3d0925c8beaa47ef3ee07ba2ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Raphael=20von=20der=20Gr=C3=BCn?= Date: Thu, 30 Aug 2018 22:02:28 +0200 Subject: [PATCH 1/4] Simplify telemetry test --- spec/cli.spec.js | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/spec/cli.spec.js b/spec/cli.spec.js index 13294b506..9a88a1111 100644 --- a/spec/cli.spec.js +++ b/spec/cli.spec.js @@ -265,16 +265,13 @@ describe('cordova cli', () => { describe('telemetry', () => { it("Test#023 : skips prompt when user runs 'cordova telemetry X'", () => { - let wasPromptShown = false; - spyOn(telemetry, 'showPrompt').and.callFake(() => { - wasPromptShown = true; - }); + spyOn(telemetry, 'showPrompt').and.returnValue(Promise.resolve()); return Promise.resolve() .then(_ => cli(['node', 'cordova', 'telemetry', 'on'])) .then(_ => cli(['node', 'cordova', 'telemetry', 'off'])) .then(() => { - expect(wasPromptShown).toBeFalsy(); + expect(telemetry.showPrompt).not.toHaveBeenCalled(); }); }); From d7b79f6ac438672d6c67bf8cca56e29f6496af95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Raphael=20von=20der=20Gr=C3=BCn?= Date: Thu, 30 Aug 2018 22:49:26 +0200 Subject: [PATCH 2/4] Factor out common telemetry stubbing --- spec/cli.spec.js | 78 +++++++++++++++++------------------------------- 1 file changed, 27 insertions(+), 51 deletions(-) diff --git a/spec/cli.spec.js b/spec/cli.spec.js index 9a88a1111..0707884d8 100644 --- a/spec/cli.spec.js +++ b/spec/cli.spec.js @@ -42,9 +42,9 @@ describe('cordova cli', () => { spyOn(console, 'log'); // Prevent accidentally turning telemetry on/off during testing - telemetry.turnOn = () => {}; - telemetry.turnOff = () => {}; - telemetry.track = () => {}; + spyOn(telemetry, 'track'); + spyOn(telemetry, 'turnOn'); + spyOn(telemetry, 'turnOff'); }); describe('options', () => { @@ -264,8 +264,16 @@ describe('cordova cli', () => { }); describe('telemetry', () => { - it("Test#023 : skips prompt when user runs 'cordova telemetry X'", () => { + beforeEach(() => { + // 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); spyOn(telemetry, 'showPrompt').and.returnValue(Promise.resolve()); + }); + + it("Test#023 : skips prompt when user runs 'cordova telemetry X'", () => { + telemetry.hasUserOptedInOrOut.and.returnValue(false); return Promise.resolve() .then(_ => cli(['node', 'cordova', 'telemetry', 'on'])) @@ -276,9 +284,7 @@ describe('cordova cli', () => { }); it("Test#024 : is NOT collected when user runs 'cordova telemetry on' while NOT opted-in", () => { - spyOn(telemetry, 'isOptedIn').and.returnValue(false); - spyOn(telemetry, 'isCI').and.returnValue(false); - spyOn(telemetry, 'track'); + telemetry.isOptedIn.and.returnValue(false); return cli(['node', 'cordova', 'telemetry', 'on']).then(() => { expect(telemetry.track).not.toHaveBeenCalled(); @@ -286,21 +292,13 @@ describe('cordova cli', () => { }); it("Test#025 : is collected when user runs 'cordova telemetry off' while opted-in", () => { - spyOn(telemetry, 'isOptedIn').and.returnValue(true); - spyOn(telemetry, 'isCI').and.returnValue(false); - spyOn(telemetry, 'track'); - return cli(['node', 'cordova', 'telemetry', 'off']).then(() => { expect(telemetry.track).toHaveBeenCalledWith('telemetry', 'off', 'via-cordova-telemetry-cmd', 'successful'); }); }); it('Test#026 : tracks platforms/plugins subcommands', () => { - spyOn(telemetry, 'isOptedIn').and.returnValue(true); - spyOn(telemetry, 'isCI').and.returnValue(false); - spyOn(telemetry, 'hasUserOptedInOrOut').and.returnValue(true); spyOn(cordova, 'platform').and.returnValue(Promise.resolve()); - spyOn(telemetry, 'track'); return cli(['node', 'cordova', 'platform', 'add', 'ios']).then(() => { expect(telemetry.track).toHaveBeenCalledWith('platform', 'add', 'successful'); @@ -309,26 +307,26 @@ describe('cordova cli', () => { it('Test#027 : shows prompt if user neither opted in or out yet', () => { spyOn(cordova, 'prepare').and.returnValue(Promise.resolve()); - spyOn(telemetry, 'hasUserOptedInOrOut').and.returnValue(false); - spyOn(telemetry, 'isCI').and.returnValue(false); - spyOn(telemetry, 'isNoTelemetryFlag').and.returnValue(false); - spyOn(telemetry, 'showPrompt').and.returnValue(Promise.resolve(false)); + telemetry.hasUserOptedInOrOut.and.returnValue(false); return cli(['node', 'cordova', 'prepare']).then(() => { expect(telemetry.showPrompt).toHaveBeenCalled(); }); }); - // note: we override telemetry timeout here so we don't need to wait 30 seconds it('Test#028 : opts-out if prompt times out AND it tracks opt-out', () => { // Remove any optOut settings that might have been saved // ... and force prompt to be shown telemetry.clear(); - spyOn(telemetry, 'hasUserOptedInOrOut').and.returnValue(false); - spyOn(telemetry, 'isCI').and.returnValue(false); - spyOn(telemetry, 'track'); - telemetry.timeoutInSecs = 1; + // We override telemetry timeout here so we don't need to wait + // 30 seconds. 0s is impossible with the current implementation. + telemetry.timeoutInSecs = 0.01; + + telemetry.isOptedIn.and.callThrough(); + telemetry.showPrompt.and.callThrough(); + telemetry.hasUserOptedInOrOut.and.returnValue(false); + return cli(['node', 'cordova', '--version']).then(() => { if (process.env.CI) { expect(telemetry.isOptedIn()).toBeTruthy(); @@ -340,11 +338,7 @@ describe('cordova cli', () => { }); it("Test#029 : is NOT collected in CI environments and doesn't prompt", () => { - spyOn(telemetry, 'hasUserOptedInOrOut').and.returnValue(true); - spyOn(telemetry, 'isOptedIn').and.returnValue(true); - spyOn(telemetry, 'isCI').and.returnValue(true); - spyOn(telemetry, 'showPrompt'); - spyOn(telemetry, 'track'); + telemetry.isCI.and.returnValue(true); return cli(['node', 'cordova', '--version']).then(() => { expect(telemetry.showPrompt).not.toHaveBeenCalled(); @@ -353,11 +347,7 @@ describe('cordova cli', () => { }); it("Test#030 : is NOT collected when --no-telemetry flag found and doesn't prompt", () => { - spyOn(telemetry, 'hasUserOptedInOrOut').and.returnValue(false); - spyOn(telemetry, 'isOptedIn').and.returnValue(true); - spyOn(telemetry, 'isCI').and.returnValue(false); - spyOn(telemetry, 'showPrompt'); - spyOn(telemetry, 'track'); + telemetry.hasUserOptedInOrOut.and.returnValue(false); return cli(['node', 'cordova', '--version', '--no-telemetry']).then(() => { expect(telemetry.showPrompt).not.toHaveBeenCalled(); @@ -366,11 +356,8 @@ describe('cordova cli', () => { }); it('Test#031 : is NOT collected if user opted out', () => { - spyOn(telemetry, 'hasUserOptedInOrOut').and.returnValue(true); - spyOn(telemetry, 'isOptedIn').and.returnValue(false); - spyOn(telemetry, 'isCI').and.returnValue(false); - spyOn(telemetry, 'showPrompt'); - spyOn(telemetry, 'track'); + telemetry.isOptedIn.and.returnValue(false); + telemetry.hasUserOptedInOrOut.and.returnValue(true); return cli(['node', 'cordova', '--version']).then(() => { expect(telemetry.showPrompt).not.toHaveBeenCalled(); @@ -379,12 +366,6 @@ describe('cordova cli', () => { }); it('Test#032 : is collected if user opted in', () => { - spyOn(telemetry, 'hasUserOptedInOrOut').and.returnValue(true); - spyOn(telemetry, 'isOptedIn').and.returnValue(true); - spyOn(telemetry, 'isCI').and.returnValue(false); - spyOn(telemetry, 'showPrompt'); - spyOn(telemetry, 'track'); - return cli(['node', 'cordova', '--version']).then(() => { expect(telemetry.showPrompt).not.toHaveBeenCalled(); expect(telemetry.track).toHaveBeenCalled(); @@ -392,12 +373,7 @@ describe('cordova cli', () => { }); it("Test#033 : track opt-out that happened via 'cordova telemetry off' even if user is NOT opted-in ", () => { - spyOn(telemetry, 'isCI').and.returnValue(false); - spyOn(telemetry, 'isOptedIn').and.returnValue(false); // same as calling `telemetry.turnOff();` - spyOn(telemetry, 'hasUserOptedInOrOut').and.returnValue(true); - spyOn(telemetry, 'track'); - - expect(telemetry.isOptedIn()).toBeFalsy(); + telemetry.isOptedIn.and.returnValue(false); return cli(['node', 'cordova', 'telemetry', 'off']).then(() => { expect(telemetry.isOptedIn()).toBeFalsy(); From 3b1aa4bf40d9429e572fdce9be715789ffa1c4af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Raphael=20von=20der=20Gr=C3=BCn?= Date: Fri, 31 Aug 2018 03:35:42 +0200 Subject: [PATCH 3/4] Prevent telemetry prompt on local test runs The telemetry prompt was shown when running tests if the following were true: - Developer has not yet opted in or out of telemetry - `!process.enc.CI` This commits fixes that by globally stubbing telemetry.showPrompt. --- spec/cli.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/cli.spec.js b/spec/cli.spec.js index 0707884d8..f2cbe0896 100644 --- a/spec/cli.spec.js +++ b/spec/cli.spec.js @@ -45,6 +45,7 @@ describe('cordova cli', () => { spyOn(telemetry, 'track'); spyOn(telemetry, 'turnOn'); spyOn(telemetry, 'turnOff'); + spyOn(telemetry, 'showPrompt').and.returnValue(Promise.resolve()); }); describe('options', () => { @@ -269,7 +270,6 @@ describe('cordova cli', () => { spyOn(telemetry, 'isCI').and.returnValue(false); spyOn(telemetry, 'isOptedIn').and.returnValue(true); spyOn(telemetry, 'hasUserOptedInOrOut').and.returnValue(true); - spyOn(telemetry, 'showPrompt').and.returnValue(Promise.resolve()); }); it("Test#023 : skips prompt when user runs 'cordova telemetry X'", () => { From 645c1bf1250ce81b38c3575f5707776d3498d8de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Raphael=20von=20der=20Gr=C3=BCn?= Date: Fri, 31 Aug 2018 12:00:58 +0200 Subject: [PATCH 4/4] Avoid printing a telemetry prompt during testing --- spec/cli.spec.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/spec/cli.spec.js b/spec/cli.spec.js index f2cbe0896..6d0e48f84 100644 --- a/spec/cli.spec.js +++ b/spec/cli.spec.js @@ -323,6 +323,9 @@ describe('cordova cli', () => { // 30 seconds. 0s is impossible with the current implementation. telemetry.timeoutInSecs = 0.01; + // Don't display the prompt + spyOn(process.stdout, 'write'); + telemetry.isOptedIn.and.callThrough(); telemetry.showPrompt.and.callThrough(); telemetry.hasUserOptedInOrOut.and.returnValue(false);