Golden master testsuite

[stevvooe]

After seeing issues like #626, it seems wise to create a golden master testsuite that contains known valid data structures and ensures that we parse and generate them correctly. Such a test suite should be done outside of the scope tools like gojsonschema to ensure they are operating correctly.

[erikh]

Maybe just some data in json files, parse and check the fields? Is that what you were thinking? I might be able to do this over an evening or weekend if that's all that's necessary.

…

-Erik

On Tue, Apr 4, 2017 at 1:45 PM Stephen Day ***@***.***> wrote: After seeing issues like #626 <#626>, it seems wise to create a golden master testsuite that contains known valid data structures and ensures that we parse and generate them correctly. Such a test suite should be done outside of the scope tools like gojsonschema to ensure they are operating correctly. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#637>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABJ6zXCuUlvfg6QeOZN9qPkJ816PseDks5rsqvUgaJpZM4MzZ2I> .

[stevvooe]

@erikh Yes, very simple. Most of the effort will be in verifying the golden master output.

[philips]

What do you mean by "golden master output"? @erikh is suggesting parsing json and checking fields while you seem to be thinking about something differently.

[stevvooe]

@philips "Golden Master" is the technique of generating hand verified files and then testing output against them. They should have the correct values and format. It is just a form of black box testing that is effective in side-stepping the kinds of issues we've seen as of late.

The technique is demonstrated here in https://github.com/golang/protobuf/blob/master/protoc-gen-go/testdata/imp.pb.go.golden.

[erikh]

I'll put a reminder in to research over the weekend what needs to be done and hopefully get back to you then with more information.

…

On Thu, Apr 13, 2017 at 1:41 PM Stephen Day ***@***.***> wrote: @philips <https://github.com/philips> "Golden Master" is the technique of generating hand verified files and then testing output against them. They should have the correct values and format. It is just a form of black box testing that is effective in side-stepping the kinds of issues we've seen as of late. The technique is demonstrated here in https://github.com/golang/protobuf/blob/master/protoc-gen-go/testdata/imp.pb.go.golden . — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#637 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABJ67Co0HgpUCGSgsH2eU50fSnSy6vcks5rvoh3gaJpZM4MzZ2I> .

[stevvooe]

@erikh Any updates?

[erikh]

ah crap; I've been buried in box issues. I'll work on this tomorrow AM. Is there urgency here? I don't want to hold anyone up.

…

On Tue, Apr 18, 2017 at 12:59 PM Stephen Day ***@***.***> wrote: @erikh <https://github.com/erikh> Any updates? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#637 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABJ68RZ1Wo_uh1Se-NlYi-DtvfsPPAwks5rxRYNgaJpZM4MzZ2I> .

[erikh]

@stevvooe how do you feel about a program that takes a directory that could be packed into an OCI-compatible image, which is read in, validated, and assumed partial, and generates the rest of the fields based on defaults or spec calculation (e.g. chainids). it then can pass that to the user for validation; we can dog food this in our own tests to download test data and reformulate to ensure no deviation (or at least, alert on it) from the spec as well; take the image, remove the json files, run our tool, validate against the original json files.

the field generation/processing could be provided either with direct code or text/template generation, not entirely sure how that would work yet, but the text/template idea was basically to allow restricted generation (e.g., with a fixed name so you always get your result with it in there).

Validation can be performed trivially by gojsonschema I think; I think that code already exists. The differentiator here is the gap-filling calculations.

The big advantages of this approach:

• "diffable" results since generation is tightly controlled
• chainIDs are easy to vet
• in theory, this should be useful to people implementing OCI image libraries both from a "this is how it's done" and a "is my stuff working" perspective...

Let me know if I'm not being clear; I'm still sorting through some of this in my head.

[stevvooe]

For the most part, we don't really need to validate fully baked image layouts. This effort should focus on getting masters together for the individual components. At this point, we've seen several reports of various problems that are internal to each data type, so we can keep the scope here relatively small.

the field generation/processing could be provided either with direct code or text/template generation, not entirely sure how that would work yet, but the text/template idea was basically to allow restricted generation (e.g., with a fixed name so you always get your result with it in there).

The idea of a golden master avoids any generation.

Validation can be performed trivially by gojsonschema I think

Again, we need to go around gojsonschema: we can see that it is not actually working or performing validation in all cases. The idea of a golden master verifies gojsonschema, as well.

You basically have a test like this:

func TestGolden(t *testing.T) {
  for _, testcase := range []struct{
    name string
    input interface{}
    expected string // can be a literal or a path
  }{
  name: "MyCase",
  input: ocispec.Manifest{...},
  expected: 
}{
    t.Run(testcase.name, func(t *testing.T) {
      p, err := json.Marshal(testcase.input)
      if err != nil { t.Fatal(err) }

      // note that it is okay to reindent the expected for consistency
      if !bytes.Equal(p, testcase.expected) { // check marshaling
        t.Fatal(...) // show a diff
      }

      // check we got back what was expected
      roundtrip := reflect.Zero(reflect.TypeOf(testcase.input)).Interface() // i think this is right
      if err := json.Unmarshal(testcase.expected, roundtrip); err != nil { t.Fatal(err) }
      if !reflect.DeepEqual(roundtrip, testcase.input) { t.Fatal(...) }
  }})
}

[vbatts]

i'm in favor moving this to post-v1.0.0 so it is not a blocker

[stevvooe]

@vbatts Agreed.