-
Notifications
You must be signed in to change notification settings - Fork 235
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Simplified onboarding experiment assignment and logging (#1036)
Users are randomly assigned to get the classic onboarding experience
('control') or a simplified onboarding experience without app setup
('treatment'.) This allocation is hard-coded in the extension; if we
want a different rollout we need to update the extension.
Once we render the login component, we consider the user has been
exposed to a specific arm of the trial and we cache this fact in local
storage.
Currently fixes the rollout at 0.5 so people running main will get
exposed to the experiment. We can bump `SIMPLIFIED_ARM_ALLOCATION` back
to zero if we want to hold back the experiment.
Testers can set a boolean property, `testing.simplified-onboarding`, to
force the extension to display the treatment (true) or control (false)
arm of the trial.
Part of #632
## Test plan
Automated test:
```
pnpm test:unit # new tests for experiment arm assigment, caching
pnpm test:e2e && pnpm test:integration # hard-coded to use the classic onboarding
# e2e tests for the new login flow TBD
```
Manual test:
1. Cody, ... menu, Sign out, sign out.
2. Control ("Other Sign in Options..." etc.) login should appear.
3. Open the VScode Output pane and change the dropdown to Cody by
Sourcegraph. This log message should appear:
```
logEvent (telemetry disabled): CodyVSCodeExtension:experiment:simplifiedOnboarding:exposed {"arm":"control","excludeFromExperiment":false}
```
4. Verify that login, etc. works.
5. Preferences: Open User Settings JSON. Add a key
`"testing.simplified-onboarding": true`. Reload VScode.
6. New login experience should appear.
7. VScode Output pane, this log message should appear:
```
simplified-onboarding: not logging exposure for testing override selection
```
8. Verify login works.- Loading branch information
1 parent
650736c
commit 4220307
Showing
12 changed files
with
379 additions
and
25 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,159 @@ | ||
| import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' | ||
| import * as vscode from 'vscode' | ||
|
|
||
| import { OnboardingExperimentArm } from '../chat/protocol' | ||
|
|
||
| import { localStorage } from './LocalStorageProvider' | ||
| import * as OnboardingExperiment from './OnboardingExperiment' | ||
|
|
||
| vi.mock('vscode', mockVScode) | ||
| vi.mock('./LocalStorageProvider', mockLocalStorage) | ||
| vi.mock('../log', mockLog) | ||
|
|
||
| function mockVScode() { | ||
| return { | ||
| UIKind: { | ||
| Desktop: 1, | ||
| Web: 42, | ||
| }, | ||
| env: { | ||
| uiKind: 1, // Desktop | ||
| }, | ||
| workspace: { | ||
| getConfiguration: () => ({ | ||
| get: () => undefined, | ||
| }), | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| function mockLog() { | ||
| return { | ||
| logDebug: () => {}, | ||
| } | ||
| } | ||
|
|
||
| function mockLocalStorage() { | ||
| return { | ||
| localStorage: { | ||
| get: () => null, | ||
| set: () => {}, | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| describe('OnboardingExperiment', () => { | ||
| const mockTelemetry = { | ||
| log: vi.fn(), | ||
| } | ||
|
|
||
| beforeEach(() => { | ||
| OnboardingExperiment.resetForTesting() | ||
| }) | ||
|
|
||
| afterEach(() => { | ||
| vi.restoreAllMocks() | ||
| }) | ||
|
|
||
| it('caches arms on exposure, not when picking them', async () => { | ||
| vi.spyOn(global.Math, 'random').mockReturnValueOnce(2) | ||
| const set = vi.spyOn(localStorage, 'set') | ||
| OnboardingExperiment.pickArm(mockTelemetry) | ||
| expect(set).not.toHaveBeenCalled() | ||
| await OnboardingExperiment.logExposure() | ||
| expect(set).toHaveBeenCalledWith('experiment.onboarding', '{"arm":0,"excludeFromExperiment":false}') | ||
| }) | ||
|
|
||
| it('randomly assigns arms', () => { | ||
| // A number less than zero will always trigger the experiment. | ||
| const random = vi.spyOn(global.Math, 'random').mockReturnValueOnce(-1) | ||
| expect(OnboardingExperiment.pickArm(mockTelemetry)).toBe(OnboardingExperimentArm.Simplified) | ||
| expect(random).toBeCalled() | ||
|
|
||
| OnboardingExperiment.resetForTesting() | ||
| // A number greater than 1 will always trigger the control arm of the trial. | ||
| random.mockReturnValueOnce(2) | ||
| expect(OnboardingExperiment.pickArm(mockTelemetry)).toBe(OnboardingExperimentArm.Classic) | ||
| }) | ||
|
|
||
| it('caches the arm in memory once picked', () => { | ||
| const localStorageGet = vi.spyOn(localStorage, 'get') | ||
| const random = vi.spyOn(global.Math, 'random') | ||
| const arm = OnboardingExperiment.pickArm(mockTelemetry) | ||
| expect(localStorageGet).toBeCalled() | ||
| expect(random).toBeCalled() | ||
|
|
||
| localStorageGet.mockReset() | ||
| random.mockReset() | ||
| expect(OnboardingExperiment.pickArm(mockTelemetry)).toBe(arm) | ||
| expect(localStorageGet).not.toBeCalled() | ||
| expect(random).not.toBeCalled() | ||
| }) | ||
|
|
||
| it('logs exposures', async () => { | ||
| vi.spyOn(global.Math, 'random').mockReturnValueOnce(-1) | ||
| expect(OnboardingExperiment.pickArm(mockTelemetry)).toBe(OnboardingExperimentArm.Simplified) | ||
| const log = vi.spyOn(mockTelemetry, 'log') | ||
| await OnboardingExperiment.logExposure() | ||
| expect(log).toHaveBeenCalledWith('CodyVSCodeExtension:experiment:simplifiedOnboarding:exposed', { | ||
| arm: 'treatment', | ||
| excludeFromExperiment: false, | ||
| }) | ||
| }) | ||
|
|
||
| it('defers to arms cached in local storage', async () => { | ||
| const localStorageGet = vi.spyOn(localStorage, 'get').mockReturnValue('{"arm":0,"excludeFromExperiment":true}') | ||
| const random = vi.spyOn(global.Math, 'random') | ||
| expect(OnboardingExperiment.pickArm(mockTelemetry)).toBe(OnboardingExperimentArm.Classic) | ||
| expect(localStorageGet).toBeCalled() | ||
| expect(random).not.toBeCalled() | ||
|
|
||
| const log = vi.spyOn(mockTelemetry, 'log') | ||
| await OnboardingExperiment.logExposure() | ||
| expect(log).toHaveBeenCalledWith('CodyVSCodeExtension:experiment:simplifiedOnboarding:exposed', { | ||
| arm: 'control', | ||
| excludeFromExperiment: true, | ||
| }) | ||
| }) | ||
|
|
||
| it('can be overridden ON with a config parameter', async () => { | ||
| vi.spyOn(vscode.workspace, 'getConfiguration').mockReturnValue({ | ||
| get: (key: string) => key === 'testing.simplified-onboarding', | ||
| } as unknown as vscode.WorkspaceConfiguration) | ||
| const localStorageGet = vi.spyOn(localStorage, 'get') | ||
| const random = vi.spyOn(global.Math, 'random') | ||
| expect(OnboardingExperiment.pickArm(mockTelemetry)).toBe(OnboardingExperimentArm.Simplified) | ||
| expect(localStorageGet).not.toBeCalled() | ||
| expect(random).not.toBeCalled() | ||
|
|
||
| const log = vi.spyOn(mockTelemetry, 'log') | ||
| await OnboardingExperiment.logExposure() | ||
| expect(log).not.toBeCalled() | ||
| }) | ||
|
|
||
| it('can be overridden OFF with a config parameter', async () => { | ||
| vi.spyOn(vscode.workspace, 'getConfiguration').mockReturnValue({ | ||
| get: () => false, | ||
| } as unknown as vscode.WorkspaceConfiguration) | ||
| const localStorageGet = vi.spyOn(localStorage, 'get') | ||
| const random = vi.spyOn(global.Math, 'random') | ||
| expect(OnboardingExperiment.pickArm(mockTelemetry)).toBe(OnboardingExperimentArm.Classic) | ||
| expect(localStorageGet).not.toBeCalled() | ||
| expect(random).not.toBeCalled() | ||
|
|
||
| const log = vi.spyOn(mockTelemetry, 'log') | ||
| await OnboardingExperiment.logExposure() | ||
| expect(log).not.toBeCalled() | ||
| }) | ||
|
|
||
| it('does not log exposures from overrides', async () => { | ||
| vi.spyOn(vscode.workspace, 'getConfiguration').mockReturnValue({ | ||
| get: (key: string) => key === 'testing.simplified-onboarding', | ||
| } as unknown as vscode.WorkspaceConfiguration) | ||
| expect(OnboardingExperiment.pickArm(mockTelemetry)).toBe(OnboardingExperimentArm.Simplified) | ||
|
|
||
| const log = vi.spyOn(mockTelemetry, 'log') | ||
| await OnboardingExperiment.logExposure() | ||
| expect(log).not.toBeCalled() | ||
| }) | ||
| }) |
Oops, something went wrong.