Add testing for vanilla and ocamlcompatible serializers #4252

pbiggar · 2022-07-07T20:26:38Z

After many wrong paths, this adds testing to the Vanilla and OCamlCompatible serializers. This adds significant clarity about what kind of changes can be safely made to the serializers. It also adds some (indirect sorta) documentation to where the serializers are used.

The tests have three parts:

everywhere we use Vanilla and OCamlCompatible serializers I mark them as allowed by calling Json.Vanilla.allow<type> "reason" (or Json.OCamlCompatible) in new initialization functions
in non-production environment, we assert that these types are have test cases added in Serialization.Tests.fs
the serialized version of these tests cases are stored in backend/serialization, and are checked against them. Any changes cause these files to change (they are checked into git)

Most of the excitement here is in Serialization.Tests.fs

We now have a really good understanding of where the vanilla and ocamlcompatible serializers are used, which should make it much easier to eradicate the ocamlcompatible one (in some future PRs)

Some things I did to make this work:

move the meat of Analysis.fs into LibAnalysis.fs, which is not a wasm project and can be integrated directly into the Tests project more easily
Make the OCamlTypes more concrete by removing the polymorphism that always used fluidExpr
Removed Apiserver.Json which was a clone of the vanilla serializer. This was a different attempt to make the serializers less accident prone, but it's no longer necessary with this
Make UI.fs globals lazy (so they run after the initialization functions)
fixed running ./scripts/build/dotnet-fsi

The annotation didn't actually do this - it only runs when reflection functions are called on the type, so we couldn't guarantee that all the serialized types were made note of, preventing us from doing regression testing on them.

(users don't have characters anyway)

…alizer This makes it much easier to debug why initializers are called

This allows the serializers initialization to run

fsharp-backend/src/LibService/Rollbar.fs

fsharp-backend/src/Prelude/Prelude.fs

config/circleci

docs/serialization.md

fsharp-backend/src/Analysis/Analysis.fs

fsharp-backend/src/LibBackend/File.fs

fsharp-backend/tests/FuzzTests/FuzzTests.fs

scripts/devcontainer/_vscode-post-start-command

fsharp-backend/tests/Tests/Serialization.Tests.fs

StachuDotNet · 2022-07-08T16:59:21Z

fsharp-backend/tests/Tests/Serialization.Tests.fs

+      let newValue = (name, f v) :: currentValue
+      data[key] <- newValue
+
+    let v<'t when 't : equality> (dataName : string) (data : 't) =


I think it's a marginal cost to rename these fns to vanilla and ocamlcompat for sake of legibility

or better yet, genVanilla and genOcaml. Even genV and genO would be better

fsharp-backend/tests/Tests/Serialization.Tests.fs

StachuDotNet

This looks good. I've noted various minor change requests, but barring those this makes me feel more secure in our serializers. (though naturally there's still work to be done for additional serializers)

StachuDotNet · 2022-07-08T17:22:15Z

fsharp-backend/src/Analysis/Analysis.fs

@@ -307,6 +66,9 @@ type EvalWorker =
  ///
  /// Once evaluated, an async call to `self.postMessage` will be made
  static member OnMessage(input : string) =
+    // We'd like to only call this once on initialization, but I can't figure out how


Here's my idea:

define static member OnInit() = ... fn

create a binding to that exposed member, and call it right before the 2+3 warmup call, here:

dark/fsharp-backend/src/Analysis/static/BlazorWorker.js

Line 927 in 427b928

const messageHandler = Module.mono_bind_static_method(

Or have you tried that and it doesn't work?

pbiggar · 2022-07-09T22:03:20Z

Adding the initializer to BlazorWorker/EvalWorker worked great, thanks!

pbiggar requested a review from StachuDotNet July 8, 2022 02:07

pbiggar marked this pull request as ready for review July 8, 2022 02:07

pbiggar added 28 commits July 8, 2022 02:08

Move values into a module

2e5a919

Rename to handle all serialization

05aa213

Generalize the binary serialization test

cf0c0ab

Remove dead code

38464f9

Make note of the serializers

ec5ede1

Annotate all the types that Json.Vanilla or Json.OCamlCompatible

dc37e4a

Add some types I missed

ed765ff

Add Analysis

70c0bd0

Better variable names

1f0e7fb

Add paket.references for new project

369e723

Fix some comments

a5ab6ee

Get serialization tests in a roughly working shape

9da643b

Add more serialization tests

b7c6783

Don't break when characters are passed through

3c7e0cd

(users don't have characters anyway)

Add better serialization documentation

9a97d4f

Formatting

83928a4

Remove old doc

9a3a53e

Add test data

c122a18

Remove extra serialization format, it's not as unsafe now

cb26b8b

Fix typo

27d8148

Improve serialization doc

744852e

No need to specific LibService here

582cf82

Move initializers closer to their use, and add a reason for the initi…

dfdccc7

…alizer This makes it much easier to debug why initializers are called

Make Ui load lazily so that it loads after the call to init()

be20081

This allows the serializers initialization to run

Also log errors

236af90

Add consistent IDs

e9dc1db

Fix problems with calling ./scripts/build/dotnet-fsi

d3b9954

pbiggar added 2 commits July 8, 2022 02:08

Remove unused types

a7e33e2

Add tests for unused types

b1a01e6

pbiggar force-pushed the paul/test-serializers branch from e47d47c to b1a01e6 Compare July 8, 2022 02:08