Fuzz all pure functions

[pbiggar]

Mark LibJwt, x509 and crypto functions as ImpurePreviewable to run in the fuzzer

[StachuDotNet]

Just to make sure I understand correctly, would this be an appropriate rephrase?
"Fuzztest LibJwt, x509 and crypto functions. You'll need to mark them as ImpurePreviewable to run in the fuzzer"

[pbiggar]

More like "Fuzz all pure functions, taking care to also test the functions marked ImpurePreviewable".

So what I'm thinking is:

• run the fuzzer again to make sure we haven't introduced any new issues
• remove the remaining limitations of the fuzzer to dig into any corner cases (see FSTODOs for that)
• also fuzz the functions marked ImpurePreviewable (x509, LibJwt, etc)
• see if there are ways to increase our confidence that code/functions work the same by expanding the fuzzer

[StachuDotNet]

Here's an analysis of all Dark fns, noting which ones can/can't be fuzztested.

I gathered the list by writing a a variant of DarkInternal::allFunctions locally which includes Previewable status (and doesn't exclude deprecated fns), and manipulating the resultant JSON into the Dynalist doc, organizing fns where they seemed appropriate.

Some notes:

• I assume toForm should be ignored, as it's marked as for demo only
• Date::today is currently marked as Pure - I'm planning as marking this as Impure
• String.htmlEscape is currently Impore - I believe it should be Pure
• while most of StaticAssets can't be fuzztested, several fns (StaticAssets::baseUrlFor, StaticAssets::fetch, StaticAssets::urlFor) should be fuzz-testable w/r/t ocaml interop, at least. Would those be appropriate to be marked as ImpurePreviewable?
• I haven't yet tested X509::pemCertificatePublicKey but understand it can be fuzztested; currently sitting in the 'unorganized' section at the bottom
• I'm still a bit confused why some functions are ImpurePreviewable rather than Pure (e.g. Crypto::md5); does ImpurePreviewable really mean "pure, but let's save it because it's a bit costly?" or am I misinterpreting?

If there are any fns in that document out of place, please let me know as I'm working my way through them.

---

Edit: I hadn't previously studied the ExecutePureFunctions in detail, because I found it a bit overwhelming.
I just gave it another look and realize I may have been misunderstanding - still processing.

[pbiggar]

I'm still a bit confused why some functions are ImpurePreviewable rather than Pure (e.g. Crypto::md5); does ImpurePreviewable really mean "pure, but let's save it because it's a bit costly?" or am I misinterpreting?

"Pure" doesn't actually mean pure, it means "this can be run in the editor via js_of_ocaml". Some things were impure in the OCaml version as they were not runnable in the editor, mostly because of their use of C/native bindings.

"ImpurePreviewable" means "this can only run on the server but is actually pure" (eg Crypto::md5) or "you can safely run this without side-effects, but it is not pure" (eg Date::now).

[pbiggar]

I gathered the list by writing a a variant of DarkInternal::allFunctions locally which includes Previewable status (and doesn't exclude deprecated fns), and manipulating the resultant JSON into the Dynalist doc, organizing fns where they seemed appropriate.

These look right.

The DB ones could possibly be fuzztested:

• db::keys, db::schema etc could be fuzzed
• db::query etc could possibly be fuzzed, unsure?
• db::set/update could probably be fuzzed to see what's in the DB after

However, probably a lower priority than other stuff.

Some notes:

• I assume toForm should be ignored, as it's marked as for demo only

Agreed.

• Date::today is currently marked as Pure - I'm planning as marking this as Impure
• String.htmlEscape is currently Impore - I believe it should be Pure

These should remain the same as OCaml until we've removed OCaml, so I'd add a CLEANUP comment to them instead.

If you're using "change them to Pure" to mean "test them as if they are pure", then yes, I agree.

• while most of StaticAssets can't be fuzztested, several fns (StaticAssets::baseUrlFor, StaticAssets::fetch, StaticAssets::urlFor) should be fuzz-testable w/r/t ocaml interop, at least. Would those be appropriate to be marked as ImpurePreviewable?

Only if it doesn't change their behaviour with respect to OCaml. Otherwise I'd add a CLEANUP to do it.

[StachuDotNet]

Closing stale PR #3654 now, but wanted to extract two threads/todos so I don't forget:

• JWT testing follow-up
• allowing Incompletes

[pbiggar]

Should we declare this done?

[StachuDotNet]

Yeah I'd say so. Always more fuzztesting to do, but separate from this specific scope.