Skip to content

[surface] Scala 3: Pass all tests on JVM#2052

Merged
xerial merged 5 commits intowvlet:masterfrom
exoego:surface-fix2
Feb 3, 2022
Merged

[surface] Scala 3: Pass all tests on JVM#2052
xerial merged 5 commits intowvlet:masterfrom
exoego:surface-fix2

Conversation

@exoego
Copy link
Copy Markdown
Collaborator

@exoego exoego commented Feb 3, 2022 •
edited
Loading

It might be quick and dirty 😅

before

✅ 97

$ DOTTY=1 sbt surfaceJVM/test:clean;surfaceJVM/test ​

[error] Failed: Total 101, Failed 4, Errors 0, Passed 97
[error] Failed tests:
[error]         wvlet.airframe.surface.RecursiveSurfaceTest
[error]         wvlet.airframe.surface.RecursiveHigherKindTypeTest
[error]         wvlet.airframe.surface.reflect.RuntimeSurfaceTest
[error]         wvlet.airframe.surface.SurfaceTest

after

✅ 101(+4)

[info] Passed: Total 101, Failed 0, Errors 0, Passed 101

exoego added 5 commits February 3, 2022 10:23
@exoego
Handle case fields as method arguments
0b2cc33 
wvlet.airframe.surface.MultipleConstructorArgsTest
wvlet.airframe.surface.ClassSurfaceTest
@exoego
Fix confusion of List[String] wand String
5b68cc9 
Fix wvlet.airframe.surface.RecursiveSurfaceTest
@exoego
Treat type parameter such as T
904f2da 
Fix  wvlet.airframe.surface.RecursiveHigherKindTypeTest
@exoego
Different key for type params to avoid cache collision
7109aff 
fix wvlet.airframe.surface.RecursiveHigherKindTypeTest
@exoego
Different key for type bounds
b64171b 
fix wvlet.airframe.surface.reflect.RuntimeSurfaceTest
fix wvlet.airframe.surface.SurfaceTest
@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 3, 2022

Codecov Report

Merging #2052 (b64171b) into master (a9b5c92) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2052   +/-   ##
=======================================
  Coverage   82.49%   82.49%           
=======================================
  Files         301      301           
  Lines       11834    11834           
  Branches      747      747           
=======================================
  Hits         9763     9763           
  Misses       2071     2071

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a9b5c92...b64171b. Read the comment docs.

xerial
xerial approved these changes Feb 3, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@xerial xerial left a comment

Choose a reason for hiding this comment

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

Thanks for finding why higher-kinded types are not processed properly.

@xerial xerial merged commit 7b0d96c into wvlet:master Feb 3, 2022
@xerial xerial mentioned this pull request Feb 3, 2022
Closed
62 tasks
@xerial xerial added this to the Scala 3 milestone Feb 3, 2022
@xerial
Copy link
Copy Markdown
Member

xerial commented Feb 3, 2022 •
edited
Loading

@exoego Note. Some tests httpJVM/test start failing after this change. Currently, we are not running the full CI for Scala 3 as we still need many fixes, so finding the impact of the change to the other modules is a bit challenging. It might be better to add a CI test for projects whose migration has been completed (e.g., logJVM/test; controlJVM/test, surfaceJVM/test, ...)

 - create a new sync client 1.06ms << error: key not found: wvlet.airframe.http.client.URLConnectionClientBase._.Resource

java.util.NoSuchElementException: key not found: wvlet.airframe.http.client.URLConnectionClientBase._.Resource
	at scala.collection.MapOps.default(Map.scala:274)
	at scala.collection.MapOps.default$(Map.scala:273)
	at scala.collection.AbstractMap.default(Map.scala:405)
	at scala.collection.MapOps.apply(Map.scala:176)
	at scala.collection.MapOps.apply$(Map.scala:175)
	at scala.collection.AbstractMap.apply(Map.scala:405)
	at wvlet.airframe.surface.package$.getCached(package.scala:11)
	at wvlet.airframe.surface.LazySurface.ref(Surfaces.scala:327)
	at wvlet.airframe.surface.LazySurface.typeArgs(Surfaces.scala:340)
	at wvlet.airframe.surface.Alias.<init>(Surfaces.scala:189)
	at wvlet.airframe.surface.Alias$.apply(Surfaces.scala:188)

@exoego exoego deleted the surface-fix2 branch February 3, 2022 21:51
@exoego
Copy link
Copy Markdown
Collaborator Author

exoego commented Feb 4, 2022 •
edited
Loading

Number of passed tests, compared to c8173e5

logJVM: 48 -> 48 
surfaceJVM: 95 -> 101 (+6)
diJVM:  82 -> 83 (+1)
codecJVM:  109 -> 106 (-3) 
httpJVM:  24 -> 24

@exoego
Copy link
Copy Markdown
Collaborator Author

exoego commented Feb 4, 2022 •
edited
Loading

That test was failing before change, just cache key became different form.

- create a new sync client 1.80ms << error: key not found: List(wvlet, airframe, http, client, URLConnectionClientBase, _.Resource)

java.util.NoSuchElementException: key not found: List(wvlet, airframe, http, client, URLConnectionClientBase, _.Resource)
        at scala.collection.MapOps.default(Map.scala:274)
        at scala.collection.MapOps.default$(Map.scala:273)
        at scala.collection.AbstractMap.default(Map.scala:405)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@xerial xerial xerial approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

Scala 3

Development

Successfully merging this pull request may close these issues.

2 participants

@exoego @xerial