opencontainers / image-spec

OCI Image Format
https://www.opencontainers.org/
Apache License 2.0
3.43k stars 633 forks source link

Golden master testsuite #637

Open stevvooe opened 7 years ago

stevvooe commented 7 years ago

After seeing issues like https://github.com/opencontainers/image-spec/issues/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 commented 7 years ago

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 notifications@github.com wrote:

After seeing issues like #626 https://github.com/opencontainers/image-spec/issues/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 https://github.com/opencontainers/image-spec/issues/637, or mute the thread https://github.com/notifications/unsubscribe-auth/AABJ6zXCuUlvfg6QeOZN9qPkJ816PseDks5rsqvUgaJpZM4MzZ2I .

stevvooe commented 7 years ago

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

philips commented 7 years ago

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 commented 7 years ago

@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 commented 7 years ago

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 notifications@github.com 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 https://github.com/opencontainers/image-spec/issues/637#issuecomment-294015937, or mute the thread https://github.com/notifications/unsubscribe-auth/AABJ67Co0HgpUCGSgsH2eU50fSnSy6vcks5rvoh3gaJpZM4MzZ2I .

stevvooe commented 7 years ago

@erikh Any updates?

erikh commented 7 years ago

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 notifications@github.com 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 https://github.com/opencontainers/image-spec/issues/637#issuecomment-294961799, or mute the thread https://github.com/notifications/unsubscribe-auth/AABJ68RZ1Wo_uh1Se-NlYi-DtvfsPPAwks5rxRYNgaJpZM4MzZ2I .

erikh commented 7 years ago

@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:

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

stevvooe commented 7 years ago

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 commented 7 years ago

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

stevvooe commented 7 years ago

@vbatts Agreed.