-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat: implement times(Double) and div(Double) #60
feat: implement times(Double) and div(Double) #60
Conversation
src/commonMain/kotlin/io/github/kevincianfarini/alchemist/type/Acceleration.kt
Outdated
Show resolved
Hide resolved
src/commonMain/kotlin/io/github/kevincianfarini/alchemist/internal/SaturatingLong.kt
Outdated
Show resolved
Hide resolved
src/commonMain/kotlin/io/github/kevincianfarini/alchemist/type/Area.kt
Outdated
Show resolved
Hide resolved
src/commonMain/kotlin/io/github/kevincianfarini/alchemist/type/Mass.kt
Outdated
Show resolved
Hide resolved
I've updated the docs, tests, and SaturatingLong impl according to the review comments! |
src/commonMain/kotlin/io/github/kevincianfarini/alchemist/type/Acceleration.kt
Outdated
Show resolved
Hide resolved
src/commonTest/kotlin/io/github/kevincianfarini/alchemist/SaturatingLongTest.kt
Show resolved
Hide resolved
src/commonMain/kotlin/io/github/kevincianfarini/alchemist/type/Volume.kt
Outdated
Show resolved
Hide resolved
Almost there. Are you interested in also contributing |
I've made the updates, and added the |
fun dividing_by_double_zero_throws() { | ||
assertFailsWith<Exception> { // ArithmeticException on most platforms; Exception on JS | ||
10L.saturated / 0.0 | ||
} | ||
assertFailsWith<IllegalArgumentException> { | ||
POSITIVE_INFINITY / 0.0 | ||
} | ||
assertFailsWith<IllegalArgumentException> { | ||
NEGATIVE_INFINITY / 0.0 | ||
} | ||
} |
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.
Looks like this test somehow isn't working for wasmJS. Also -- if the exception types differ per platform we could use assertFails
instead of assertFailsWith
.
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.
RuntimeError: divide by zero
at <alchemist_test>.kotlin.Long__div-impl (wasm://wasm/<alchemist_test>-004f0842:wasm-function[2259]:0x3521d)
at <alchemist_test>.io.github.kevincianfarini.alchemist.internal.SaturatingLong__div-impl (wasm://wasm/<alchemist_test>-004f0842:wasm-function[3351]:0x45efe)
at <alchemist_test>.io.github.kevincianfarini.alchemist.internal.SaturatingLong__div-impl (wasm://wasm/<alchemist_test>-004f0842:wasm-function[3352]:0x45f12)
at <alchemist_test>.io.github.kevincianfarini.alchemist.internal.SaturatingLong__div-impl (wasm://wasm/<alchemist_test>-004f0842:wasm-function[3354]:0x45f40)
at <alchemist_test>.io.github.kevincianfarini.alchemist.SaturatingLongTest.dividing_by_double_zero_throws (wasm://wasm/<alchemist_test>-004f0842:wasm-function[4931]:0x67af5)
at <alchemist_test>.io.github.kevincianfarini.alchemist.test fun$io.github.kevincianfarini.alchemist test fun$SaturatingLongTest test fun$dividing_by_double_zero_throws test fun.invoke (wasm://wasm/<alchemist_test>-004f0842:wasm-function[4651]:0x5b25e)
at <alchemist_test>.kotlin.test.TeamcityAdapter$test$lambda.invoke (wasm://wasm/<alchemist_test>-004f0842:wasm-function[4226]:0x573d6)
at <alchemist_test>.kotlin.test.TeamcityAdapterWithPromiseSupport.runOrScheduleNextWithResult (wasm://wasm/<alchemist_test>-004f0842:wasm-function[4108]:0x558e5)
at <alchemist_test>.kotlin.test.TeamcityAdapterWithPromiseSupport.runOrScheduleNextWithResult (wasm://wasm/<alchemist_test>-004f0842:wasm-function[4109]:0x55923)
at <alchemist_test>.kotlin.test.TeamcityAdapter.test (wasm://wasm/<alchemist_test>-004f0842:wasm-function[4249]:0x58f4c)
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'm happy to get this over the finish line if you want @sargunv. Let me know!
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.
Go for it! If not, I'll get to it during the weekend
I unfortunately had some weird issues trying to push directly to your branch, so I opened another PR with all of your changes and some of the remaining tweaks. |
Fixes #53