Skip to content

[vitest-pool-workers] STOR-5343: Add Durable Object eviction helpers#14398

Merged
edmundhung merged 4 commits into
mainfrom
apeacock/vitest-evict-do
Jun 25, 2026
Merged

[vitest-pool-workers] STOR-5343: Add Durable Object eviction helpers#14398
edmundhung merged 4 commits into
mainfrom
apeacock/vitest-evict-do

Conversation

@apeacock1991

@apeacock1991 apeacock1991 commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Adds two new helpers to vitest-pool-workers that enable the testing of evictions for Durable Objects. It adds two new helpers:

  • evictDurableObject(stub) just evicts a single DO
  • evictAllDurableObjects evicts all DOs

@changeset-bot

changeset-bot Bot commented Jun 23, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 8c04e9a

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

This PR includes changesets to release 1 package
Name Type
@cloudflare/vitest-pool-workers Patch

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

@github-actions

github-actions Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

⚠️ Issues found

.changeset/evict-durable-objects.md:

  • Version type: Adding evictDurableObject and evictAllDurableObjects helpers is a new feature/API addition. Per the changeset guidelines, new features should use minor, not patch.

@ask-bonk

ask-bonk Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

The analysis is complete. Let me assess the actionable issues:

The key observations:

  1. evictDurableObject doesn't call assertSameIsolate(stub) while its docstring claims it "can only be used with stubs pointing to Durable Objects defined in the main worker." This is the most defensible concern, but workerdUnsafe.evict is a runtime call that may handle cross-isolate stubs differently than runInDurableObject (which depends on the in-memory actionResults map). Eviction doesn't use that map, so the constraint may not actually apply the same way. This is uncertain — not a clear bug.

  2. Test comment at line 44 ("Persist count = 2") is slightly inaccurate but trivial/formatting-adjacent.

  3. The PR body states "This is reliant on a workerd change that is not pushed remote yet." This means the new tests will fail in CI until that workerd change lands. That's a process note, not a code bug per the review scope.

Let me double-check the test for the bulk eviction — after evictAllDurableObjects(), the comment says count reloads from storage (1) so next increment yields 2. The first fetch persisted count=1, then in-memory set to 999 (not persisted). After eviction, reload 1, next fetch increments to 2. Correct.

None of these rise to the level of a clear logic bug, security issue, or backward-compatibility violation. The same-isolate concern is genuinely uncertain (eviction semantics differ from runInDurableObject), and flagging it would be speculative. The comment inaccuracy is trivial. The workerd-dependency is already disclosed by the author.

LGTM

github run

@pkg-pr-new

pkg-pr-new Bot commented Jun 23, 2026

Copy link
Copy Markdown
@cloudflare/autoconfig

npm i https://pkg.pr.new/@cloudflare/autoconfig@14398

create-cloudflare

npm i https://pkg.pr.new/create-cloudflare@14398

@cloudflare/deploy-helpers

npm i https://pkg.pr.new/@cloudflare/deploy-helpers@14398

@cloudflare/kv-asset-handler

npm i https://pkg.pr.new/@cloudflare/kv-asset-handler@14398

miniflare

npm i https://pkg.pr.new/miniflare@14398

@cloudflare/pages-shared

npm i https://pkg.pr.new/@cloudflare/pages-shared@14398

@cloudflare/unenv-preset

npm i https://pkg.pr.new/@cloudflare/unenv-preset@14398

@cloudflare/vite-plugin

npm i https://pkg.pr.new/@cloudflare/vite-plugin@14398

@cloudflare/vitest-pool-workers

npm i https://pkg.pr.new/@cloudflare/vitest-pool-workers@14398

@cloudflare/workers-auth

npm i https://pkg.pr.new/@cloudflare/workers-auth@14398

@cloudflare/workers-editor-shared

npm i https://pkg.pr.new/@cloudflare/workers-editor-shared@14398

@cloudflare/workers-utils

npm i https://pkg.pr.new/@cloudflare/workers-utils@14398

wrangler

npm i https://pkg.pr.new/wrangler@14398

commit: 8c04e9a

@apeacock1991 apeacock1991 changed the title [vitest-pool-workers] Add Durable Object eviction helpers [vitest-pool-workers] STOR-5343: Add Durable Object eviction helpers Jun 23, 2026

@jtolio jtolio left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

How much of the TypeScript definitions come from the Workerd automated output?

@apeacock1991

Copy link
Copy Markdown
Contributor Author

How much of the TypeScript definitions come from the Workerd automated output?

None 😅 I will try tomorrow, as I had no idea that existed!

@apeacock1991 apeacock1991 marked this pull request as ready for review June 24, 2026 17:01
@workers-devprod workers-devprod requested review from a team and james-elicx and removed request for a team June 24, 2026 17:02
@workers-devprod

workers-devprod commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Codeowners approval required for this PR:

  • ✅ @cloudflare/wrangler
Show detailed file reviewers

@apeacock1991

Copy link
Copy Markdown
Contributor Author

@james-elicx this is waiting on a workerd bump before it can be merged, but be helpful to get a pass at the code before then

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

@apeacock1991 apeacock1991 force-pushed the apeacock/vitest-evict-do branch from 3297fcb to 392560d Compare June 25, 2026 08:18

@workers-devprod workers-devprod left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Codeowners reviews satisfied

@github-project-automation github-project-automation Bot moved this from Untriaged to Approved in workers-sdk Jun 25, 2026
Comment thread .changeset/evict-durable-objects.md Outdated
Comment thread fixtures/vitest-pool-workers-examples/durable-objects/test/eviction.test.ts Outdated
@apeacock1991 apeacock1991 force-pushed the apeacock/vitest-evict-do branch from a238d7f to 8c04e9a Compare June 25, 2026 11:29

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

Open in Devin Review

Comment thread .changeset/evict-durable-objects.md
@edmundhung edmundhung merged commit c5014cc into main Jun 25, 2026
64 checks passed
@github-project-automation github-project-automation Bot moved this from Approved to Done in workers-sdk Jun 25, 2026
@edmundhung edmundhung deleted the apeacock/vitest-evict-do branch June 25, 2026 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants