Skip to content
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

Update tests to run for JS targets #584

Merged
merged 3 commits into from
Jun 30, 2024

Conversation

OptimumCode
Copy link
Contributor

Hi, this PR allows running tests for JS targets.

The old version of tests had the following problems:

  • It is incorrect to compare NaN to another NaN using == operator. For Java Float.NaN == Float.NaN returns false (but Objects.equals(Float.NaN, Float.NaN) returns true , which is why tests work for Java). For JS it is a bit different.

    NaN compares unequal (via ==, !=, ===, and !==) to any other value — including to another NaN value.

    For NaN comparison, equals call was replaced with isNaN() method call.

  • The hex and octal numbers are valid floating point numbers for JS (because under the hood Kotlin delegates conversion to JS dynamic cast). This is the toDouble implementation for JS platform

    public actual fun String.toDouble(): Double = (+(this.asDynamic())).unsafeCast<Double>().also {
        if (it.isNaN() && !this.isNaN() || it == 0.0 && this.isBlank())
            numberFormatError(this)
    }

    Tests for those values are ignored for JS platform.

@sschuberth please, take a look at the PR when you have time. Thank you!

@sschuberth
Copy link
Contributor

@sschuberth please, take a look at the PR when you have time.

Done, though I'm not sure why, as I have no approval rights on this repo 😉

@OptimumCode
Copy link
Contributor Author

Thank you for your review @sschuberth!
I wanted to tag @charleskorn but he does not appear in the completion list for some reason, and I accidentally tagged you instead)) don't know why - probably because it is a bit late here already)

@sschuberth
Copy link
Contributor

I accidentally tagged you instead

No worries, I hope my review was useful still 😸

@OptimumCode
Copy link
Contributor Author

OptimumCode commented Jun 27, 2024

No worries, I hope my review was useful still 😸

It definitely was! Thanks again

@charleskorn
Copy link
Owner

Thanks @OptimumCode and @sschuberth!

@charleskorn charleskorn merged commit c23a906 into charleskorn:main Jun 30, 2024
1 check passed
@OptimumCode OptimumCode deleted the enable-js-test branch June 30, 2024 07:10
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