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.

Sign up for GitHub

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

Jump to bottom

http-tests: allow to compile with Scala 3 with 3.0-migration option #4107

Merged
merged 3 commits into from
Apr 25, 2022
Merged

http-tests: allow to compile with Scala 3 with 3.0-migration option #4107

merged 3 commits into from
Apr 25, 2022

Conversation

jchyb
Copy link
Contributor

@jchyb jchyb commented Apr 13, 2022

This also includes akka-http-spray-json and akka-http-scala-xml , on which the tests depend.
Because of changes to type inference, some tests had to be slightly adjusted (beyond defining types explicitly). They all compile, though running tests locally concludes with a somewhat confusing message:

[info] ScalaTest
[info] Run completed in 40 seconds, 36 milliseconds.
[info] Total number of tests run: 1273
[info] Suites: completed 65, aborted 0
[info] Tests: succeeded 1273, failed 0, canceled 0, ignored 0, pending 40
[info] All tests passed.
[info] multi-jvm
[info] akka.http.AkkaHttpServerLatencyMultiNodeSpec
[error] Failed: Total 1432, Failed 0, Errors 0, Passed 1432, Pending 40
[error] (akka-http-tests / Test / test) sbt.TestsFailedException: Tests unsuccessful

3.0-migration compiler option was used - there are still some errors when compiling without it that I was not able to iron out yet.

References #4079, tasks akka-http-tests (with 3.0-migration option), akka-http-spray-json, akka-http-scala-xml

jchyb
jchyb commented Apr 13, 2022
View reviewed changes
akka-http-tests/src/test/scala/akka/http/scaladsl/server/EntityStreamingSpec.scala Outdated
Comment on lines 281 to 290
complete {
measurementsSubmitted.map(n => Map("msg" -> s"""Total metrics received: $n"""))
implicit val preciseFormat: RootJsonFormat[Map[String, String]] = mapFormat // workaround for Scala 3
ToResponseMarshallable(measurementsSubmitted.map((n: Int) => Map[String, String]("msg" -> s"""Total metrics received: $n""")))
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This part is interesting I think - without the additional implicit brought closer in scope the compiler would claim to have found conflicting RootJsonFormats in CollectionConverters - they were iterableFormat and immIterableFormat instead of mapFormat. Not sure why that is yet.

@jchyb jchyb changed the title http-tests: Make them compile with Scala 3. with 3.0-migration http-tests: allow to compile with Scala 3 with 3.0-migration option Apr 13, 2022
jrudolph
jrudolph approved these changes Apr 14, 2022
View reviewed changes
Copy link
Contributor

@jrudolph jrudolph left a comment

Choose a reason for hiding this comment

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

LGTM, I think we can live with the few warts.

akka-http-tests/src/test/scala/akka/http/scaladsl/server/SizeLimitSpec.scala Outdated
@@ -230,7 +230,7 @@ class SizeLimitSpec extends AnyWordSpec with Matchers with RequestBuilding with

private def chunkedEntityOfSize(size: Int) = HttpEntity.Chunked(ContentTypes.`text/plain(UTF-8)`, byteSource(size).map(Chunk(_)))
private def nonChunkedEntityOfSize(size: Int): MessageEntity = HttpEntity.Default(ContentTypes.`text/plain(UTF-8)`, size, byteSource(size))
private def entityOfSize(size: Int) = HttpEntity(ContentTypes.`text/plain(UTF-8)`, "0" * size)
private def entityOfSize(size: Int) = HttpEntity(ContentTypes.`text/plain(UTF-8)`, "0" * (size))
Copy link
Contributor

Choose a reason for hiding this comment

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

Weird, what's the error message otherwise?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, sorry! This is a leftover I missed when cleaning up commits. This is one of the places that error out when running without 3.0-migration option and I was trying to figure out what was going on. I'll go through the PR once again to see if there are more of those. Thank you for catching that.

akka-http-tests/src/test/scala/akka/http/scaladsl/server/util/TupleOpsSpec.scala Show resolved Hide resolved
akka-http-tests/src/test/scala/akka/http/scaladsl/server/directives/RouteDirectivesSpec.scala Outdated Show resolved Hide resolved
@jrudolph jrudolph mentioned this pull request Apr 14, 2022
Merged
22 tasks
@jrudolph
Copy link
Contributor

They all compile, though running tests locally concludes with a somewhat confusing message:

Let's see what CI says about this...

@jchyb jchyb force-pushed the scala3/tests branch 2 times, most recently from 983d026 to c4e2c49 Compare April 14, 2022 14:07
jchyb
jchyb commented Apr 14, 2022
View reviewed changes
Copy link
Contributor Author

@jchyb jchyb left a comment

Choose a reason for hiding this comment

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

@jrudolph Thank you for catching the Scala 2 issues, I have updated the PR hopefully making it work on both as well as removing the some previous spurious changes.

akka-http-tests/src/test/scala/akka/http/scaladsl/server/EntityStreamingSpec.scala Outdated Show resolved Hide resolved
akka-http-tests/src/test/scala/akka/http/scaladsl/server/directives/RouteDirectivesSpec.scala Outdated Show resolved Hide resolved
@jrudolph @jchyb
Update akka-http-tests/src/test/scala/akka/http/scaladsl/server/direc…
8ef755e 
…tives/RouteDirectivesSpec.scala

Try to fix SprayJsonSupport problems for both Scala 2 and 3
@jrudolph
Copy link
Contributor

Thanks a lot @jchyb!

@jrudolph jrudolph merged commit 471a0f6 into akka:scala-3 Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jrudolph jrudolph jrudolph approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jchyb @jrudolph