-
Notifications
You must be signed in to change notification settings - Fork 784
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Generalized Runtime Docs / (First round) Bun Support #3219
base: master
Are you sure you want to change the base?
Generalized Runtime Docs / (First round) Bun Support #3219
Conversation
…e.js, browser info
…usage and a dedicated Bun section
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is very nice to see this happening! I've been implicitly using bun in tevm for the evm blockchain and state packages for a while without issues
- uses: actions/checkout@v3 | ||
with: | ||
submodules: recursive | ||
- uses: oven-sh/setup-bun@v1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- uses: oven-sh/setup-bun@v1 | |
- uses: oven-sh/setup-bun@v1 | |
with: | |
bun-version: 1.0.24 |
Might want to pin the version so there is no ambiguity if a pr caused a regression or bun version changed it.
Alternatively consider just echoing the bun version in the next step of this test so if you are debugging a workflow you know exactly which version of bun it used
# working-directory: packages/client | ||
|
||
- name: Test Common | ||
run: bunx vitest run test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is actually using bun. Because vitest is an executable bun should default to using node here. I think you need to pass in the --bun
flag to use bun and you might have issues related more to vitest than bun.
run: bunx vitest run test | |
run: bun run --bun vitest run test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Docs on this are here https://bun.sh/docs/cli/run#bun
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, that is the "tip of the year"! 🙏 🙂
I was already wondering why things are so smooth. 😂 That is the missing link I needed!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to note that this is not yet working (using bun 1.0.26). Tested with common
(counter test with the original version worked), new version just hangs before any output.
Not dropping here for you to debug, might just be our homework to do with the bun updates adjusting our code base. But just to drop the information here.
@roninjin10 Thanks a lot for the feedback, that helps a lot! I wanted to wait for our "self-contained examples" work (see some other now merged in PRs), so we basically have made all examples self-contained, so that they can now run "on their own" with all necessary dependencies included and stuff, the examples are now also included in a dedicated So this makes |
Resolved merge conflict here and updated this via UI |
Update: gave this another try. There is an issue open on Vitest support over on Bun which is still open: oven-sh/bun#4145 and states that there is something in Bun still missing for this to work: So seems it is still too early to try on this route. |
What was an interesting experiment though: I managed to switch over our transaction test runner (which takes quite some time, >30 sec) to use the Bun native test runner (see docs) by doing some few replacements: // import { assert, describe, it } from 'vitest'
import { describe, expect, test } from 'bun:test'
// assert.ok(!txIsValid, `Transaction should be invalid on ${forkName}`)
expect(!txIsValid).toBeTruthy()
// assert.ok(hashAndSenderAreCorrect && txIsValid,`Transaction should be valid on ${forkName}`)
expect(hashAndSenderAreCorrect && txIsValid).toBeTruthy()
// assert.ok(shouldBeInvalid, `Transaction should be invalid on ${forkName}`)
expect(shouldBeInvalid).toBeTruthy()
//assert.fail(`Transaction should be valid on ${forkName}`) (no replacement) This produces the following output: Test execution times are 31.11s with Bun and 32.97s with Vitest. |
This PR adds a generalized "Runtimes" information section to the docs combining info for Node.js and browser (rewritten to be more specific) and then also adds a new "Bun" sub section there (we might want to add Deno as well).
I've done this in an examplaric way for one library, open for a first round of feedback, then I would expand (the same section/structure) to the other libraries (except client).
I've also added a
simple.ts
example, I would want to have one for each package, since this just makes sense in general.For
Bun
I have also added a new GH Actions script. This is meant to be run nightly. After a first round I would comment out all non-working packages. For the working ones we can add the new "Bun Support" runtime section.