Skip to content

Conversation

@nekevss
Copy link
Member

@nekevss nekevss commented Nov 12, 2025

This adds the general logic from FractionToDouble to handle edge case Duration::total operations.

We still need to figure out a good way to test this in Rust, but all the Duration suite tests pass when ran locally in Boa.

@nekevss nekevss requested a review from Manishearth November 12, 2025 03:52
Copy link
Contributor

@Manishearth Manishearth left a comment

Choose a reason for hiding this comment

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

Shouldn't be hard to just write the existing tests in Rust.

@nekevss
Copy link
Member Author

nekevss commented Nov 12, 2025

I have one already from another branch, but it has a bug and is creating the wrong expected value. So I need to clean it up a bit before adding it.

@Manishearth
Copy link
Contributor

I pushed a test that seems to work. I used the expected values from the JS tests that were failing.

@Manishearth Manishearth merged commit 55fb816 into main Nov 12, 2025
8 checks passed
@Manishearth Manishearth deleted the impl-double-double branch November 12, 2025 06:20
@nekevss
Copy link
Member Author

nekevss commented Nov 12, 2025

Ah, that will work. I was hoping to have a full version of the test262 test. But I'm not sure it's going to be possible without bringing in boa_string as a dev-dependency for access to a JsString type.

@Manishearth
Copy link
Contributor

Ah, that will work. I was hoping to have a full version of the test262 test. But I'm not sure it's going to be possible without bringing in boa_string as a dev-dependency for access to a JsString type.

Wouldn't be a good test then IMO since that behavior itself could be buggy.

@nekevss
Copy link
Member Author

nekevss commented Nov 12, 2025

Wouldn't be a good test then IMO since that behavior itself could be buggy.

It would be the best representation of the test262 test in Rust. But I don't necessarily disagree.

In any case, we have at least a few engines running the test262 suite to help flag potential regressions. It would be nice to catch it in our tests.

@nekevss nekevss mentioned this pull request Dec 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants