Add remaining j.u.function interfaces #4365
Conversation
javalib/src/main/scala/java/util/function/LongUnaryOperator.scala
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Thanks for the PR.
All of those new interfaces should have tests in https://github.com/scala-js/scala-js/tree/master/test-suite/shared/src/test/scala/org/scalajs/testsuite/javalib/util/function. There are already tests for the existing JDK function types in that directory, that you can take inspiration from.
dbbde57 to
6140951
Compare
|
This PR is ready to get reviewed |
...hared/src/test/scala/org/scalajs/testsuite/javalib/util/function/IntBinaryOperatorTest.scala
Outdated
Show resolved
Hide resolved
| val f = new IntToDoubleFunction { | ||
| override def applyAsDouble(value: Int): Double = value.toDouble / 11d | ||
| } | ||
| assertEquals(f.applyAsDouble(3), 0.272, 0.001) |
There was a problem hiding this comment.
Take something that rounds properly so you don' need the epsilon?
There was a problem hiding this comment.
I think epsilon is needed since JUnit deprecated assertEquals(double expected, double actual) and encourage to use assertEquals(double expected, double actual, double delta) .
https://junit.org/junit4/javadoc/4.12/org/junit/Assert.html#assertEquals(double,%20double)
There was a problem hiding this comment.
Yes but we can use a 0.0 value for the epsilon. We often do that.
...shared/src/test/scala/org/scalajs/testsuite/javalib/util/function/IntUnaryOperatorTest.scala
Outdated
Show resolved
Hide resolved
...shared/src/test/scala/org/scalajs/testsuite/javalib/util/function/ToLongBiFunctionTest.scala
Outdated
Show resolved
Hide resolved
|
Note that the consumer tests do not test exception behavior. IMO this is OK, given how simple the implementation is. @sjrd opinions? |
Should be fine. |
|
@gzm0 Commits were squashed. |
|
Oh sorry. Maybe that wasn't clear. I meant the fix stuff should go into the body not the subject. Like this it is very hard to read the commit subject. |
|
@sjrd any further input / requests? |
Closes #4227, #4228, and #4229