schematic icon indicating copy to clipboard operation
schematic copied to clipboard

fine tune property tests

Open mhanberg opened this issue 2 years ago • 6 comments

mhanberg avatar May 24 '23 17:05 mhanberg

@clark-lindsay i think that the input |> unify |> dump == input might be more specifically something like

inputs that are not supersets of the schematic always unify and dump to itself

i believe the problem with the current property is that if an input map has a key that is not in the schematic, when you unify and then dump, it won't be present in the result.

so a separate property could be "non specified keys are discarded", which then would check that the result is a subset of the input

mhanberg avatar May 24 '23 17:05 mhanberg

also, copying from the discord

i wrote a property that produced a failure for a schematic that is

list(oneof([map(%{}), int(), map(%{...stuff...})])

but, a map schematic with an empty blueprint will unify any input to an empty map (it has no keys, and discards any keys that are not in the blueprint) so in a case like this image the input is successfully unified but when it's dumped back, it won't match the input because all of the keys in the input were discarded so i think technically the property is wrong but it makes me wonder if that is confusing behavior that would be a weird schematic to write tho

mhanberg avatar May 24 '23 17:05 mhanberg

i see how it's failing, but i'm having a hard time working out how to adjust the tests. i see it in three-ish ways:

  • the current property is replaced wholesale with a (very similar) property test that asserts that input |> unify |> dump results in a value that is equivalent in all "nodes" to the input, except for lists that contain maps because we may have ended up unify-ing a non-empty map through a oneof that contained a map(%{}) schematic and thus dropped all the keys?
  • or would it be simpler to adjust the generators in some way so that that first property test doesn't have to deal with this funny case, and then we have a second property that "dump discards unspecified keys"
  • change the source code such that a dump for the maximally permissive map(%{}) schematic passes all of the data through, maybe with an option on the schematic to instead discard unspecified keys?

if i'm not quite aligned with you then i'd appreciate being redirected 😅 i'm gonna start looking into the first option

clark-lindsay avatar May 24 '23 20:05 clark-lindsay

I think we want to replace the existing property with a similar one whose generated input space is restricted.

mhanberg avatar May 24 '23 21:05 mhanberg

i just roughed out a quick and dirty version where i left everything the same except i ensured that when a list(...) schematic is created from data during generation, and the list contains a map of any kind, the schematic generation does not use a blueprint to describe the map (it will only use the explicit enumeration of key schematics and value schematics), and that seems to do the trick. just finished with max_runs: 50_000, which i'm sure my computer loved.

so this would change the input space of the generated schematics but not the input space for the generated data.

i think it would also work if the generation was adjusted such that the schematic generated for the map literal %{} was always the K-V style schematic

clark-lindsay avatar May 24 '23 22:05 clark-lindsay

i was wrong on that second assumption, actually. very wrong.

the property fails pretty quickly when we allow any map/1 schematics that use a "blueprint" as part of the oneof/1 argument to list/1. nesting is fine, though. my first "rough draft" approach would allow something like list(oneof([map(keys: int(), values: map(%{"foo" => "bar", ...})])) to be generated

clark-lindsay avatar May 24 '23 23:05 clark-lindsay