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

Move tests to correct folder #61

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

lenguyenthanh
Copy link
Contributor

I think we moved this test file into a wrong folder, so it stops running ( that why we thought it passes). But it is actually still failed.

@nomisRev
Copy link
Member

Damn, that's too bad :(
Good we have a failing test now, hopefully we can fix it easily.

@nomisRev
Copy link
Member

Moving to Kotlin 1.6.0 fixes the issue for me. I'm also going to attempt to use the workaround, since this is internal anyway.

@nomisRev
Copy link
Member

@lenguyenthanh this is the fix following the tickets on the trackers.

@Serializable(with = ReferencedSerializer::class)
public open class Referenced<out A> internal constructor() {
  public data class Ref(public val value: Reference) : Referenced<Nothing>()
  public data class Other<A>(val value: A) : Referenced<A>()

  public fun <B> map(f: (A) -> B): Referenced<B> =
    when (this) {
      is Other -> Other(f(value))
      is Ref -> this
      else -> TODO("Impossible.")
    }
}

Test locally, and in downstream project and it fixes the issue for me.
I can add/push this fix on your branch later if you like so we can see this test pass.

@lenguyenthanh
Copy link
Contributor Author

Great thanks @nomisRev ! I will try it local and push it to this branch.

@lenguyenthanh
Copy link
Contributor Author

lenguyenthanh commented Mar 24, 2022

It worked locally, and open-api module is also working now for the first time 🎉

@nomisRev
Copy link
Member

According to YouTrack this should be fixed in Kotlin 1.6.20, which is currently in M1.
So perhaps we should wait a bit to merge this PR. Hopefully in the next week we can introduce alpha releases like in Arrow, and maybe Kotlin 1.6.20 will be released by then.

Off-topic

@lenguyenthanh Are you depending on the SNAPSHOT? I also found they have an incorrect name.

"io.arrow-kt:ktor-server:0.1.2-SNAPSHOT"
"io.arrow-kt:openapi-docs:0.1.2-SNAPSHOT"

instead of

"io.arrow-kt:arrow-endpoint-ktor-server:0.1.2-SNAPSHOT"
"io.arrow-kt:arrow-endpoint-openapi-docs:0.1.2-SNAPSHOT"

@nomisRev
Copy link
Member

Thanks for reporting and helping looking into this @lenguyenthanh ❤️

@lenguyenthanh
Copy link
Contributor Author

Yeah, I agree with your approach, no need to rush to merge this PR.

I'm still using my Jitpack version. But I will try to use the snapshot version soon. Thanks for the head up, @nomisRev !

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