[cli] add telemetry tracking to `project add` #12340

onsclom · 2024-10-22T19:11:04Z

Telemetry for vc project add:

1 argument (name)
0 flags
0 options

changeset-bot · 2024-10-22T19:11:09Z

🦋 Changeset detected

Latest commit: 68f24f8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
vercel	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

jeffsee55 · 2024-10-22T22:05:04Z

packages/cli/test/unit/commands/project/add.test.ts

+      const project: Project = await client.fetch(`/v8/projects/test-project`);
+      expect(project).toBeDefined();
+
+      expect(client.telemetryEventStore).toHaveTelemetryEvents([


Instead of creating a new test that doesn't differ from the one above you can just put this assertion into the first test

Good point, was copying this pattern from https://github.com/vercel/vercel/pull/12194/files#diff-e6a0a76a9158df1b8578fa4bbd0a67a25019133076cdae07cc0dfeb5e99c8695R40. Since this pattern already exists and it provides better test failing messages, maybe we keep it like that?

Lemme short circuit the conversation since I swear it always goes exactly like this:

"Or shift the duplicated code to a beforeEach and only focus on the expectations of client.telemetryEventStore."
"Then we're just running more tests and that takes more time."
"But we get focused failures! Can we just use beforeAll"
"beforeAll just ends up causing really confusing state issues that are hard to debug"

This is a unit test that appears to hit mocked endpoints. I imagine it's super fast. If true, I don't think we need to worry about adding an extra test here.

I gave my approval because both options are correct but also wrong.

@EndangeredMassa it still takes time to set create all those objects and execute the code under test! Eventually it adds up.

(stage whisper) Sean, it's your line.

In general, the patterns I've come to like in tests:

Use beforeAll is little as possible for reasons Trek already stated.

Arrange, Act, Assert is a nice mental model, and beforeEach should only be used for the Arrange

Multiple assertions are good. They are a way of saying, when this thing happens, here are all of the side effects or outcomes. You can read them in one place. I hadn't given much thought to the fact that you get a better error message when you split them up, good point @onsclom

@jeffsee55 it also depends on the framework. Some have expect syntax like expect(actual).toSomethingSomething(expected, failureExplanationIfNot) or similar. @alex_neo/jest-expect-message has this for jest so should work with vite as well I think?

If we had and used that consistently I'd be happy with many assertions. Plus it lets you add better failure reasons generally.

Good:

it('project user count and team user account identical', () => { project.addUser(user); expect(project.userCount).toEqual(team.userCount); }); // "Failed 'project user count and team user account identical'. Expected 3, got 2" // is super actionable.

Bad:

it('adding a user', () => { project.addUser(user); expect(project.userCount).toEqual(team.userCount); }); // "Failed 'adding a user'. Expected 3, got 2" // requires my faulty skull bacon to do work

But, also good:

it('adding a user', () => { project.addUser(user); expect(project.userCount, 'project user count and team user account must be identical').toEqual(team.userCount); }); // "Failed 'adding a user': Project user count and team user account must be identical. Expected 3, got 2" // back to actionable.

trek · 2024-10-23T14:46:48Z

packages/cli/test/unit/commands/project/add.test.ts

+      client.setArgv('project', 'add', 'test-project');
+      await projects(client);
+
+      const project: Project = await client.fetch(`/v8/projects/test-project`);


I know this was copied from above but... what is it doing?

Good catch! Looks like it's just testing the useProject mock. I'll make a followup PR to remove this

…ng-for-vercel-project-add

Telemetry for `vc project add`: 1 argument (name) 0 flags 0 options

@trek

These asserts only test the `useProject` mock, not `project add` logic. Thanks for catching this @trek! #12340 (comment)

This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## vercel@37.13.0 ### Minor Changes - Add telemetry for `vercel logs` ([#12386](#12386)) - Add telemetry for `vercel env pull` ([#12368](#12368)) - add telemetry for `vercel env rm` ([#12384](#12384)) - [cli] add subcommand tracking for `dns` group ([#12375](#12375)) - [cli] add telemetry for `vercel certs ls` ([#12383](#12383)) - [cli] add telemetry tracking to vercel inspect ([#12367](#12367)) - Add telemetry for `vercel certs remove` ([#12394](#12394)) - Add telemetry for `vercel integration list` ([#12404](#12404)) - add telemetry tracking to `env add` ([#12357](#12357)) - track standard environments in `vc env add` ([#12385](#12385)) - [cli] add telemetry for `vercel target ls` ([#12352](#12352)) - [cli] add telemetry tracking for `vercel domains ls` ([#12400](#12400)) - [cli] add `VERCEL_ENV` and `VERCEL` to process to simulate runtime ([#12358](#12358)) - Add telemetry for `vercel domains move` ([#12411](#12411)) - [cli] add telemetry tracking to `git connect` and `git disconnect` ([#12373](#12373)) - Add telemetry to `vercel domains buy` ([#12413](#12413)) - [cli] add telemetry tracking to `project list` ([#12339](#12339)) - Add telemetry for `vercel domains add` ([#12409](#12409)) - Add telemetry to `vercel domains rm` ([#12410](#12410)) - Add telemetry for `vercel list` ([#12364](#12364)) - [cli] add subcommand tracking for `integration` group ([#12377](#12377)) - Add telemetry for `vercel certs add` ([#12391](#12391)) - [cli] add subcommand tracking for `domains` group ([#12374](#12374)) - [cli] add telemetry tracking to `project add` ([#12340](#12340)) - Add telemetry for `vercel integration open` ([#12408](#12408)) - Add telemetry for `vercel init` ([#12371](#12371)) - [cli] add subcommand tracking for `integration` group ([#12377](#12377)) - [cli] add telemetry tracking for `vercel dns import` ([#12379](#12379)) - [cli] add telemetry for `vercel dns ls` ([#12380](#12380)) - Add telemetry for `vercel env ls` ([#12392](#12392)) - Add telemetry for `vercel bisect` ([#12362](#12362)) - [cli] add telemetry for `vercel dns remove` ([#12381](#12381)) - Add telemetry for `vercel integration add` ([#12406](#12406)) - [cli] add telemetry tracking for vercel dev invocations ([#12349](#12349)) - [cli] add subcommand tracking for `certs` group ([#12376](#12376)) - [cli] add telemetry for `vercel redeploy` ([#12353](#12353)) - [cli] add telemetry for `vercel domains inspect` ([#12407](#12407)) - Add telemetry for `vercel certs issue` ([#12401](#12401)) - Add telemetry for `vercel link` ([#12360](#12360)) ### Patch Changes - Remove stray file ([#12363](#12363)) - add metrics to dns add ([#12414](#12414)) - Add telemetry to the `teams` subcommands. Telemetry collection is not currently enabled and when it is, will be a major version bump for the CLI. ([#12346](#12346)) - add telemetry to `vc pull` ([#12387](#12387)) - [cli] refactor Output usage ([#12305](#12305)) - extract pull invocation from build ([#12372](#12372)) - extract vc pull logic from entrypoint ([#12378](#12378)) - Updated dependencies \[[`13bb443f2b0791c7bd402447fcb540cffd0b6eae`](13bb443)]: - @vercel/remix-builder@2.2.13 ## @vercel/remix-builder@2.2.13 ### Patch Changes - Update `@remix-run/dev` fork to v2.13.1 ([#12334](#12334)) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

@trek

These asserts only test the `useProject` mock, not `project add` logic. Thanks for catching this @trek! vercel/vercel#12340 (comment)

This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## vercel@37.13.0 ### Minor Changes - Add telemetry for `vercel logs` ([#12386](vercel/vercel#12386)) - Add telemetry for `vercel env pull` ([#12368](vercel/vercel#12368)) - add telemetry for `vercel env rm` ([#12384](vercel/vercel#12384)) - [cli] add subcommand tracking for `dns` group ([#12375](vercel/vercel#12375)) - [cli] add telemetry for `vercel certs ls` ([#12383](vercel/vercel#12383)) - [cli] add telemetry tracking to vercel inspect ([#12367](vercel/vercel#12367)) - Add telemetry for `vercel certs remove` ([#12394](vercel/vercel#12394)) - Add telemetry for `vercel integration list` ([#12404](vercel/vercel#12404)) - add telemetry tracking to `env add` ([#12357](vercel/vercel#12357)) - track standard environments in `vc env add` ([#12385](vercel/vercel#12385)) - [cli] add telemetry for `vercel target ls` ([#12352](vercel/vercel#12352)) - [cli] add telemetry tracking for `vercel domains ls` ([#12400](vercel/vercel#12400)) - [cli] add `VERCEL_ENV` and `VERCEL` to process to simulate runtime ([#12358](vercel/vercel#12358)) - Add telemetry for `vercel domains move` ([#12411](vercel/vercel#12411)) - [cli] add telemetry tracking to `git connect` and `git disconnect` ([#12373](vercel/vercel#12373)) - Add telemetry to `vercel domains buy` ([#12413](vercel/vercel#12413)) - [cli] add telemetry tracking to `project list` ([#12339](vercel/vercel#12339)) - Add telemetry for `vercel domains add` ([#12409](vercel/vercel#12409)) - Add telemetry to `vercel domains rm` ([#12410](vercel/vercel#12410)) - Add telemetry for `vercel list` ([#12364](vercel/vercel#12364)) - [cli] add subcommand tracking for `integration` group ([#12377](vercel/vercel#12377)) - Add telemetry for `vercel certs add` ([#12391](vercel/vercel#12391)) - [cli] add subcommand tracking for `domains` group ([#12374](vercel/vercel#12374)) - [cli] add telemetry tracking to `project add` ([#12340](vercel/vercel#12340)) - Add telemetry for `vercel integration open` ([#12408](vercel/vercel#12408)) - Add telemetry for `vercel init` ([#12371](vercel/vercel#12371)) - [cli] add subcommand tracking for `integration` group ([#12377](vercel/vercel#12377)) - [cli] add telemetry tracking for `vercel dns import` ([#12379](vercel/vercel#12379)) - [cli] add telemetry for `vercel dns ls` ([#12380](vercel/vercel#12380)) - Add telemetry for `vercel env ls` ([#12392](vercel/vercel#12392)) - Add telemetry for `vercel bisect` ([#12362](vercel/vercel#12362)) - [cli] add telemetry for `vercel dns remove` ([#12381](vercel/vercel#12381)) - Add telemetry for `vercel integration add` ([#12406](vercel/vercel#12406)) - [cli] add telemetry tracking for vercel dev invocations ([#12349](vercel/vercel#12349)) - [cli] add subcommand tracking for `certs` group ([#12376](vercel/vercel#12376)) - [cli] add telemetry for `vercel redeploy` ([#12353](vercel/vercel#12353)) - [cli] add telemetry for `vercel domains inspect` ([#12407](vercel/vercel#12407)) - Add telemetry for `vercel certs issue` ([#12401](vercel/vercel#12401)) - Add telemetry for `vercel link` ([#12360](vercel/vercel#12360)) ### Patch Changes - Remove stray file ([#12363](vercel/vercel#12363)) - add metrics to dns add ([#12414](vercel/vercel#12414)) - Add telemetry to the `teams` subcommands. Telemetry collection is not currently enabled and when it is, will be a major version bump for the CLI. ([#12346](vercel/vercel#12346)) - add telemetry to `vc pull` ([#12387](vercel/vercel#12387)) - [cli] refactor Output usage ([#12305](vercel/vercel#12305)) - extract pull invocation from build ([#12372](vercel/vercel#12372)) - extract vc pull logic from entrypoint ([#12378](vercel/vercel#12378)) - Updated dependencies \[[`e4304c4526f6062d23b7fdb381d9bb24c7431a0d`](vercel/vercel@e4304c4)]: - @vercel/remix-builder@2.2.13 ## @vercel/remix-builder@2.2.13 ### Patch Changes - Update `@remix-run/dev` fork to v2.13.1 ([#12334](vercel/vercel#12334)) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

onsclom added 2 commits October 22, 2024 10:27

Track vc project add

877df89

Add changeset

feecea1

onsclom marked this pull request as ready for review October 22, 2024 19:38

onsclom requested review from TooTallNate, EndangeredMassa, trek, jeffsee55 and erikareads as code owners October 22, 2024 19:38

jeffsee55 requested changes Oct 22, 2024

View reviewed changes

trek approved these changes Oct 23, 2024

View reviewed changes

trek reviewed Oct 23, 2024

View reviewed changes

jeffsee55 approved these changes Oct 23, 2024

View reviewed changes

onsclom added the pr: automerge Automatically merge the PR when checks pass label Oct 23, 2024

Merge branch 'main' into austinmerrick/zero-2685-add-telemetry-tracki…

68f24f8

…ng-for-vercel-project-add

vercel bot deployed to Preview October 23, 2024 21:56 View deployment

kodiakhq bot merged commit 804b94f into main Oct 23, 2024
145 checks passed

kodiakhq bot deleted the austinmerrick/zero-2685-add-telemetry-tracking-for-vercel-project-add branch October 23, 2024 22:18

vercel-release-bot mentioned this pull request Oct 23, 2024

Version Packages #12345

Merged

onsclom mentioned this pull request Oct 23, 2024

Remove asserts on mock function #12359

Merged

trek pushed a commit that referenced this pull request Oct 24, 2024

[cli] add telemetry tracking to project add (#12340)

1f030e7

Telemetry for `vc project add`: 1 argument (name) 0 flags 0 options

kodiakhq bot pushed a commit that referenced this pull request Oct 24, 2024

Remove asserts on mock function (#12359)

7129682

These asserts only test the `useProject` mock, not `project add` logic. Thanks for catching this @trek! #12340 (comment)

QuietCraftsmanship pushed a commit to QuietCraftsmanship/Vercel that referenced this pull request Jul 6, 2025

Remove asserts on mock function (#12359)

a87207c

These asserts only test the `useProject` mock, not `project add` logic. Thanks for catching this @trek! vercel/vercel#12340 (comment)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[cli] add telemetry tracking to `project add` #12340

[cli] add telemetry tracking to `project add` #12340

Uh oh!

onsclom commented Oct 22, 2024

Uh oh!

changeset-bot bot commented Oct 22, 2024 •

edited

Loading

Uh oh!

jeffsee55 Oct 22, 2024 •

edited

Loading

Uh oh!

onsclom Oct 22, 2024

Uh oh!

trek Oct 23, 2024

Uh oh!

EndangeredMassa Oct 23, 2024

Uh oh!

trek Oct 23, 2024

Uh oh!

trek Oct 23, 2024

Uh oh!

trek Oct 23, 2024

Uh oh!

jeffsee55 Oct 23, 2024

Uh oh!

trek Oct 23, 2024 •

edited

Loading

Uh oh!

trek Oct 23, 2024

Uh oh!

onsclom Oct 23, 2024

Uh oh!

Uh oh!

Uh oh!

[cli] add telemetry tracking to project add #12340

[cli] add telemetry tracking to project add #12340

Uh oh!

Conversation

onsclom commented Oct 22, 2024

Uh oh!

changeset-bot bot commented Oct 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

jeffsee55 Oct 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

trek Oct 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

[cli] add telemetry tracking to `project add` #12340

[cli] add telemetry tracking to `project add` #12340

changeset-bot bot commented Oct 22, 2024 •

edited

Loading

jeffsee55 Oct 22, 2024 •

edited

Loading

trek Oct 23, 2024 •

edited

Loading