Skip to content

Akka.FSharp: need to re-enable discriminated union (DU) serialization tests #5194

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

Closed
Aaronontheweb opened this issue Aug 12, 2021 · 1 comment · Fixed by #5195 or #5196
Closed

Akka.FSharp: need to re-enable discriminated union (DU) serialization tests #5194

Aaronontheweb opened this issue Aug 12, 2021 · 1 comment · Fixed by #5195 or #5196

Comments

@Aaronontheweb
Copy link
Member

Version Information
Version of Akka.NET? v1.4.23
Which Akka.NET Modules? Akka.FSharp

Describe the bug
https://stackoverflow.com/questions/68741561/akka-unable-to-send-discriminated-unions-as-messages-in-f#comment121522181_68741561 - looks like default DU serialization isn't working with Newtonsoft.Json per this user's report.

As it so happens, our DU serialization tests are out of date and commented out:

//
//[<Fact>]
//let ``can serialize and deserialize discriminated unions over remote nodes using wire serializer`` () =
// let remoteConfig port =
// sprintf """
// akka {
// actor {
// ask-timeout = 5s
// provider = "Akka.Remote.RemoteActorRefProvider, Akka.Remote"
// serialization-bindings {
// "System.Object" = wire
// }
// }
// remote {
// dot-netty.tcp {
// port = %i
// hostname = localhost
// }
// }
// }
// """ port
// |> Configuration.parse
//
// use server = System.create "server-system" (remoteConfig 9911)
// use client = System.create "client-system" (remoteConfig 0)
//
// let aref =
// spawne client "a-1" <@ actorOf2 (fun mailbox msg ->
// match msg with
// | C("a-11", B(11, "a-12")) -> mailbox.Sender() <! msg
// | _ -> mailbox.Unhandled msg) @>
// [SpawnOption.Deploy (Deploy(RemoteScope (Address.Parse "akka.tcp://server-system@localhost:9911")))]
// let msg = C("a-11", B(11, "a-12"))
// let response = aref <? msg |> Async.RunSynchronously
// response
// |> equals msg

I think a lot of this stems from the Akka.NET v1.3 push and the fact that F# lagged behind a bit on .NET Core support at the time. Now that everything is pretty reliable there for the most part we should reintroduce these tests as part of our suite.

Actual behavior
Not sure, but it looks like the DU content is lost in translation.

Expected behavior
DUs should be serializable via:

  • Newtonsoft.Json
  • Hyperion

With all of its contents correctly preserved.

@Aaronontheweb
Copy link
Member Author

Whoops, not done yet. Just had to re-enable some F# platform support in the Akka.FSharp.Tests project.

Aaronontheweb added a commit to Aaronontheweb/akka.net that referenced this issue Aug 12, 2021
Arkatufus pushed a commit that referenced this issue Aug 13, 2021
* added reproduction for DU serialization

close #5194

* exposed `exprSerializationSupport` to public

* validated that record-based serialization works out of the box

* added dedicated serialization specs

* cleaned up RemoteSpecs and SerializationSpecs

Now checking for whether or not manifests are emitted

* added hardened test case to validate that DU serialization does not work

* added spec that proves this is a Newtonsoft.Json issue

* restored common.props

* disabled JSON.NET repro spec

* migrated to using Hyperion for DU tests

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