Skip to content

Conversation

@jasnell
Copy link
Collaborator

@jasnell jasnell commented Dec 18, 2025

Had claude port a handful of the easier-to-port tests and provide a checklist for doing the rest. Some will require manual figuring out to port, most likely can be automated with claude.

PR is tests and doc only. No functional changes. As long as CI passes they should be good. Once existing tests have been ported, a separate analysis of test coverage will be performed to determine what is missing.


Current state visible to Cloudflare employees: https://docs.google.com/document/d/178N1WCas0PLO4nLnr597daR5XI8NRo3lRQW3G7mOvHI/edit?tab=t.0

@codspeed-hq

This comment was marked as outdated.

@anonrig
Copy link
Member

anonrig commented Dec 18, 2025

Did you validate that these are 1 to 1 replica of the ew-tests? It's hard to review. Knowing that you've validated will ease the review process.

@jasnell
Copy link
Collaborator Author

jasnell commented Dec 18, 2025

@anonrig ...Please go ahead with whatever additional changes you feel are necessary here. Feel free to push additional commits. Before you do so, it might be easier to merge the other PR into this one to save some rebasing trouble later.

@anonrig anonrig force-pushed the jasnell/port-some-internal-streams-tests branch from f9aba30 to 33674b0 Compare December 18, 2025 21:16
@anonrig anonrig force-pushed the jasnell/port-some-internal-streams-tests branch from 33674b0 to dec9219 Compare December 18, 2025 21:21
@anonrig anonrig force-pushed the jasnell/port-some-internal-streams-tests branch 3 times, most recently from 3902808 to 41647a0 Compare December 19, 2025 20:15
Copy link
Collaborator Author

@jasnell jasnell left a comment

Choose a reason for hiding this comment

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

This likely needs a bit more of a looking at to make sure all of the ported tests are actually from ew-tests and not just incorrectly copied from already existing workerd tests.

@anonrig anonrig force-pushed the jasnell/port-some-internal-streams-tests branch 4 times, most recently from 1cb76ab to 9c630fe Compare December 22, 2025 16:08
@anonrig anonrig requested a review from danlapid December 22, 2025 16:10
@anonrig anonrig requested a review from guybedford December 22, 2025 16:10
Copy link
Collaborator

@harrishancock harrishancock left a comment

Choose a reason for hiding this comment

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

I've gotten partway through pipe-streams-test, but need to give my brain a rest.

Most of my comments so far are non-blocking observations or nits, I think, but there are a few real issues.

@jasnell
Copy link
Collaborator Author

jasnell commented Dec 29, 2025

For those reviewing.. just as an FYI... @anonrig has taken this PR over.

@anonrig
Copy link
Member

anonrig commented Dec 29, 2025

Fixing conflicts before addressing the comments.

@anonrig anonrig force-pushed the jasnell/port-some-internal-streams-tests branch 2 times, most recently from d59f41b to bbcd7e0 Compare December 30, 2025 15:47
@anonrig anonrig force-pushed the jasnell/port-some-internal-streams-tests branch from bbcd7e0 to b759d3a Compare December 30, 2025 15:47
@anonrig
Copy link
Member

anonrig commented Dec 30, 2025

@guybedford @harrishancock would you mind reviewing this? i'd like to land this asap so that we can iteratively improve the test coverage.

@anonrig anonrig enabled auto-merge (squash) December 30, 2025 18:00
@anonrig anonrig merged commit 205258d into main Dec 30, 2025
20 checks passed
@anonrig anonrig deleted the jasnell/port-some-internal-streams-tests branch December 30, 2025 18:03
danlapid pushed a commit that referenced this pull request Dec 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants