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

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

Merged
merged 5 commits into from
Feb 3, 2022
Merged

Conversation

exoego
Copy link
Collaborator

@exoego exoego commented Feb 3, 2022

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

wvlet.airframe.surface.MultipleConstructorArgsTest
wvlet.airframe.surface.ClassSurfaceTest
Fix wvlet.airframe.surface.RecursiveSurfaceTest
Fix  wvlet.airframe.surface.RecursiveHigherKindTypeTest
fix wvlet.airframe.surface.RecursiveHigherKindTypeTest
fix wvlet.airframe.surface.reflect.RuntimeSurfaceTest
fix wvlet.airframe.surface.SurfaceTest
@codecov
Copy link

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.

Copy link
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
62 tasks
@xerial xerial added this to the Scala 3 milestone Feb 3, 2022
@xerial
Copy link
Member

xerial commented Feb 3, 2022

@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
Collaborator Author

exoego commented Feb 4, 2022

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
Collaborator Author

exoego commented Feb 4, 2022

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants