Skip to content

Conversation

jeffsee55
Copy link
Contributor

When some of our test mocks intentionally respond with a 500, that response takes several seconds because the client's fetch function automatically retries the request.

But since we're mocking these requests we should just automatically fail.

Copy link

changeset-bot bot commented Oct 23, 2024

🦋 Changeset detected

Latest commit: 001bc9f

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

This PR includes changesets to release 0 packages

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

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

trek
trek previously approved these changes Oct 23, 2024
@jeffsee55 jeffsee55 added the pr: automerge Automatically merge the PR when checks pass label Oct 23, 2024
@@ -183,7 +183,7 @@ describe('rollback', () => {
`Fetching deployment "${previousDeployment.id}" in ${previousDeployment.creator?.username}`
);
// we need to wait a super long time because fetch will return on 500
Copy link
Contributor

@trek trek Oct 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can remove this comment now too. It wasn't even terribly accurate.

@kodiakhq kodiakhq bot merged commit f018976 into main Oct 23, 2024
145 checks passed
@kodiakhq kodiakhq bot deleted the jeffsee/zero-2639-investigate-vercelvercel-test-mock-500-responses-not branch October 23, 2024 23:12
trek pushed a commit that referenced this pull request Oct 24, 2024
When some of our test mocks intentionally respond with a 500, that response takes several seconds because the [client's `fetch` function](https://github.com/vercel/vercel/blob/7e8fbd20e01135513ad28936155b078650732d37/packages/cli/src/util/client.ts#L173-L174) automatically retries the request. 

But since we're mocking these requests we should just automatically fail.
kodiakhq bot pushed a commit that referenced this pull request Oct 29, 2024
For those of us who use a debugger, the test suit exiting early is frustrating. You can get around this by setting this value yourself or running with `--testTimeout` in the terminal but adding this behavior globally seems like a nice quality of life change.

I couldn't find a 100% guaranteed way to detect if node is in debug mode but the rough consensus was detecting  `debugPort` covered the largest number of cases.

I audited all the uses of `vi.setConfig({ testTimeout });` in `packages/cli` and only found one that looked like it could definitely be removed. The use case it covered was negated by #12354.

The others mostly came from 413706d or had a comment explaining their need.
QuietCraftsmanship pushed a commit to QuietCraftsmanship/Vercel that referenced this pull request Jul 6, 2025
For those of us who use a debugger, the test suit exiting early is frustrating. You can get around this by setting this value yourself or running with `--testTimeout` in the terminal but adding this behavior globally seems like a nice quality of life change.

I couldn't find a 100% guaranteed way to detect if node is in debug mode but the rough consensus was detecting  `debugPort` covered the largest number of cases.

I audited all the uses of `vi.setConfig({ testTimeout });` in `packages/cli` and only found one that looked like it could definitely be removed. The use case it covered was negated by vercel/vercel#12354.

The others mostly came from vercel/vercel@fa07756 or had a comment explaining their need.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: automerge Automatically merge the PR when checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants