[Merged by Bors] - Added a Boa runtime #2743

Razican · 2023-03-25T23:42:39Z

This Pull Request fixes/closes #718.

It changes the following:

Adds a new boa_runtime crate, that will only include console for now
Changes the boa_cli crate to use the new boa_runtime crate for the console, instead of the console feature of boa_engine
Removes the console feature in boa_engine
Adds a new boa_testing helper crate with some useful functions for testing boa. This part duplicates the code from boa_engine, but I could not make boa_engine work with this crate as a dependency due to circular dependencies. Maybe doing it a bit generic could work, but didn't have enough time to check it.

To be checked: wether the WASM example works as expected with the console.

github-actions · 2023-03-25T23:53:51Z

Test262 conformance changes

Test result	main count	PR count	difference
Total	94,591	94,593	+2
Passed	73,210	73,210	0
Ignored	17,530	17,532	+2
Failed	3,851	3,851	0
Panics	0	0	0
Conformance	77.40%	77.39%	-0.00%

codecov · 2023-03-25T23:56:21Z

Codecov Report

Merging #2743 (28c9992) into main (49e39d4) will decrease coverage by 0.05%.
The diff coverage is 33.05%.

@@            Coverage Diff             @@
##             main    #2743      +/-   ##
==========================================
- Coverage   50.99%   50.95%   -0.05%     
==========================================
  Files         419      419              
  Lines       41858    41874      +16     
==========================================
- Hits        21345    21335      -10     
- Misses      20513    20539      +26

Impacted Files	Coverage Δ
boa_cli/src/main.rs	`0.76% <0.00%> (-0.04%)`	⬇️
boa_engine/src/builtins/mod.rs	`100.00% <ø> (ø)`
boa_engine/src/context/mod.rs	`45.00% <0.00%> (-0.46%)`	⬇️
boa_engine/src/vm/call_frame/mod.rs	`88.88% <0.00%> (-11.12%)`	⬇️
boa_engine/src/vm/code_block.rs	`60.65% <0.00%> (-0.23%)`	⬇️
boa_examples/src/bin/classes.rs	`0.00% <0.00%> (ø)`
boa_examples/src/bin/futures.rs	`0.00% <0.00%> (ø)`
boa_examples/src/bin/loadstring.rs	`0.00% <0.00%> (ø)`
boa_runtime/src/console/mod.rs	`38.11% <11.32%> (ø)`
boa_engine/src/lib.rs	`78.20% <100.00%> (ø)`
... and 1 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

nekevss · 2023-03-26T01:08:03Z

boa_runtime/src/lib.rs

@@ -0,0 +1,73 @@
+//! Example runtime for Boa
+//!
+//! This crate contains an example runtime for the `boa_engine` crate, so that it can be used as a


Maybe this is a larger discussion, but should we be referring to this as an example runtime or as boa's basic runtime implementation and encourage implementors to use it as a template/build on it for their own needs?

It only has one builtin for now, so I'm not sure at this point 😅 but I'm open to it, of course

Neither am I lol I mainly mention it because the crate is named boa_runtime but it only has console in it. Since there have been issues submitted around a setTimeout implementation, someone may see the crate and immediately think: "oh, it's a runtime" and then get confused. If you're fine with it, then I'm good to merge this.

For now, I would say, let's merge it, and add timeout and all this stuff to it slowly, see how it evolves :)

jasonwilliams · 2023-03-26T10:00:00Z

Missed opportunity to call it Boa Time!?

HalidOdat

Looks good to me!

Just a small nitpick :)

boa_testing/Cargo.toml

nekevss

Looks good to me :)

jedel1043

I don't think having a public boa_testing crate is a good idea, since the API of that will change constantly while we work on the engine.

I'd very much prefer to move the tests of console into boa_testing and skip publishing the crate, maybe allowing us to migrate our integration tests to that crate in the future.

Razican · 2023-03-30T20:30:01Z

I don't think having a public boa_testing crate is a good idea, since the API of that will change constantly while we work on the engine.

I'd very much prefer to move the tests of console into boa_testing and skip publishing the crate, maybe allowing us to migrate our integration tests to that crate in the future.

I have skipped publishing of the crate. About moving console tests there, we could do it, but then I would like to have them under #[cfg(test)] so that we keep cargo test working.

jedel1043

All good then!

jedel1043

Oops, typo on the attribute

boa_testing/Cargo.toml

jedel1043 · 2023-03-31T03:35:14Z

Now that I think about it, cargo also considers dev dependencies to publish crates, so we cannot exclude boa_testing and also use it as a dev dependency on a published crate.

I know this because I ran cargo publish -p boa_runtime --dry-run and it returned:

no matching package named `boa_testing` found
  location searched: registry `crates-io`

even if boa_testing is only a dev dependency. We have to move the tests from boa_runtime to boa_testing if we don't want to publish it.

Razican · 2023-03-31T10:41:39Z

Now that I think about it, cargo also considers dev dependencies to publish crates, so we cannot exclude boa_testing and also use it as a dev dependency on a published crate.

I know this because I ran cargo publish -p boa_runtime --dry-run and it returned:
no matching package named `boa_testing` found
  location searched: registry `crates-io`
even if boa_testing is only a dev dependency. We have to move the tests from boa_runtime to boa_testing if we don't want to publish it.

Hmmm, I'm also not fully confident of having "boa_testing" and "boa_tester" as crates, can confuse people. I'll try to make the crate not depend on boa_engine, and if I can't, then I would say we should duplicate the code in boa_runtime, and improve the architecture in the future if needed.

jedel1043 · 2023-04-03T15:53:58Z

Hmmm, I'm also not fully confident of having "boa_testing" and "boa_tester" as crates, can confuse people. I'll try to make the crate not depend on boa_engine, and if I can't, then I would say we should duplicate the code in boa_runtime, and improve the architecture in the future if needed.

Maybe we could just name it 'tests'? If the crate won't be published anyways, it shouldn't matter if it doesn't have the 'boa_' prefix. It also gives a pretty strong indicator that this crate is not published, only used internally.

Razican · 2023-04-12T09:47:35Z

Hmmm, I'm also not fully confident of having "boa_testing" and "boa_tester" as crates, can confuse people. I'll try to make the crate not depend on boa_engine, and if I can't, then I would say we should duplicate the code in boa_runtime, and improve the architecture in the future if needed.

Maybe we could just name it 'tests'? If the crate won't be published anyways, it shouldn't matter if it doesn't have the 'boa_' prefix. It also gives a pretty strong indicator that this crate is not published, only used internally.

For now, I've gone through the route of duplicating the code in the boa_runtime crate. This should work while we find a better solution for this where we reduce code duplication, publishing works well, and it still code coverage and cargo test work.

Feel free to review it again, as I have rebased the code :)

Cargo.toml

Razican · 2023-04-15T18:13:51Z

I re-based and ran "cargo update", which updated ICU to 1.2.0. Unfortunately, they have broken backwards compatibility, first with this: unicode-org/icu4x#3332, but then, I replaced the imports, and I now get a bunch of compile errors.

So, it seems that 1.2.0 is not backwards compatible with 1.1.0. I have pinned 1.1.0 for now.

This should probably be reviewed & merged :) Missing @jedel1043's confirmation

jedel1043

We can merge as it is, we'll try to deduplicate the tests logic in the future. Nice work!

HalidOdat · 2023-04-18T20:49:26Z

bors r+

bors · 2023-04-18T20:53:11Z

Merge conflict.

Razican · 2023-04-23T21:11:55Z

I have rebased this, and reverted bitflags, since bitflags 2.2.0 was yanked. I think we can merge it now (if all tests pass)

HalidOdat

Looks good to me! Lets merge this! :)

HalidOdat · 2023-04-24T02:34:12Z

bors r+

This Pull Request fixes/closes #718. It changes the following: - Adds a new `boa_runtime` crate, that will only include `console` for now - Changes the `boa_cli` crate to use the new `boa_runtime` crate for the console, instead of the `console` feature of `boa_engine` - Removes the `console` feature in `boa_engine` - Adds a new `boa_testing` helper crate with some useful functions for testing `boa`. This part duplicates the code from `boa_engine`, but I could not make `boa_engine` work with this crate as a dependency due to circular dependencies. Maybe doing it a bit generic could work, but didn't have enough time to check it. To be checked: wether the WASM example works as expected with the console.

bors · 2023-04-24T02:50:42Z

Pull request successfully merged into main.

Build succeeded:

Razican added builtins PRs and Issues related to builtins/intrinsics API labels Mar 25, 2023

Razican added this to the v0.17.0 milestone Mar 25, 2023

Razican requested review from jasonwilliams, HalidOdat, RageKnify, raskad, jedel1043 and nekevss March 25, 2023 23:42

nekevss reviewed Mar 26, 2023

View reviewed changes

HalidOdat approved these changes Mar 27, 2023

View reviewed changes

boa_testing/Cargo.toml Outdated Show resolved Hide resolved

Razican force-pushed the runtime branch from 6a05432 to b545934 Compare March 30, 2023 15:18

Razican requested a review from nekevss March 30, 2023 15:20

nekevss approved these changes Mar 30, 2023

View reviewed changes

jedel1043 requested changes Mar 30, 2023

View reviewed changes

jedel1043 approved these changes Mar 30, 2023

View reviewed changes

jedel1043 requested changes Mar 30, 2023

View reviewed changes

boa_testing/Cargo.toml Outdated Show resolved Hide resolved

jedel1043 mentioned this pull request Apr 5, 2023

Create new Runtime API #2783

Closed

Razican force-pushed the runtime branch from d01a144 to c2b1334 Compare April 12, 2023 09:36

Razican requested a review from jedel1043 April 12, 2023 09:45

Razican commented Apr 12, 2023

View reviewed changes

Cargo.toml Show resolved Hide resolved

Razican force-pushed the runtime branch from cc38810 to 4ce93b3 Compare April 15, 2023 17:58

jedel1043 approved these changes Apr 15, 2023

View reviewed changes

Added a Boa runtime

ec57b9e

Razican force-pushed the runtime branch from f47f4b9 to ec57b9e Compare April 23, 2023 21:03

Fixed comment, updated Test262

28c9992

HalidOdat approved these changes Apr 24, 2023

View reviewed changes

bors bot changed the title ~~Added a Boa runtime~~ [Merged by Bors] - Added a Boa runtime Apr 24, 2023

bors bot closed this Apr 24, 2023

bors bot deleted the runtime branch April 24, 2023 02:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Merged by Bors] - Added a Boa runtime #2743

[Merged by Bors] - Added a Boa runtime #2743

Razican commented Mar 25, 2023

github-actions bot commented Mar 25, 2023 •

edited

Loading

codecov bot commented Mar 25, 2023 •

edited

Loading

nekevss Mar 26, 2023

Razican Mar 26, 2023

nekevss Mar 28, 2023

Razican Mar 30, 2023

jasonwilliams commented Mar 26, 2023

HalidOdat left a comment

nekevss left a comment

jedel1043 left a comment •

edited

Loading

Razican commented Mar 30, 2023

jedel1043 left a comment

jedel1043 left a comment

jedel1043 commented Mar 31, 2023

Razican commented Mar 31, 2023

jedel1043 commented Apr 3, 2023

Razican commented Apr 12, 2023

Razican commented Apr 15, 2023

jedel1043 left a comment

HalidOdat commented Apr 18, 2023

bors bot commented Apr 18, 2023

Razican commented Apr 23, 2023

HalidOdat left a comment

HalidOdat commented Apr 24, 2023

bors bot commented Apr 24, 2023

[Merged by Bors] - Added a Boa runtime #2743

[Merged by Bors] - Added a Boa runtime #2743

Conversation

Razican commented Mar 25, 2023

github-actions bot commented Mar 25, 2023 • edited Loading

Test262 conformance changes

codecov bot commented Mar 25, 2023 • edited Loading

Codecov Report

nekevss Mar 26, 2023

Choose a reason for hiding this comment

Razican Mar 26, 2023

Choose a reason for hiding this comment

nekevss Mar 28, 2023

Choose a reason for hiding this comment

Razican Mar 30, 2023

Choose a reason for hiding this comment

jasonwilliams commented Mar 26, 2023

HalidOdat left a comment

Choose a reason for hiding this comment

nekevss left a comment

Choose a reason for hiding this comment

jedel1043 left a comment • edited Loading

Choose a reason for hiding this comment

Razican commented Mar 30, 2023

jedel1043 left a comment

Choose a reason for hiding this comment

jedel1043 left a comment

Choose a reason for hiding this comment

jedel1043 commented Mar 31, 2023

Razican commented Mar 31, 2023

jedel1043 commented Apr 3, 2023

Razican commented Apr 12, 2023

Razican commented Apr 15, 2023

jedel1043 left a comment

Choose a reason for hiding this comment

HalidOdat commented Apr 18, 2023

bors bot commented Apr 18, 2023

Razican commented Apr 23, 2023

HalidOdat left a comment

Choose a reason for hiding this comment

HalidOdat commented Apr 24, 2023

bors bot commented Apr 24, 2023

github-actions bot commented Mar 25, 2023 •

edited

Loading

codecov bot commented Mar 25, 2023 •

edited

Loading

jedel1043 left a comment •

edited

Loading