Skip to content

Remove Golden Tests from the test suite #8162

Open
@vmg

Description

@vmg

Since we've upgraded our ProtoBuf package version in #8075, the new serializer to JSON and ProtoText inserts random whitespace on its output that prevents byte-wise comparison.

Our test suite makes extensive use of "golden tests" that compare these outputs. In order to complete the upgrade successfully, I had to disable the randomness in the serializer.

This is explained in detail in the following comment:

// DisableProtoBufRandomness disables the random insertion of whitespace characters when
// serializing Protocol Buffers in textual form (both when serializing to JSON or to ProtoText)
//
// Since the introduction of the APIv2 for Protocol Buffers, the default serializers in the
// package insert random whitespace characters that don't change the meaning of the serialized
// code but make byte-wise comparison impossible. The rationale behind this decision is as follows:
//
// "The ProtoBuf authors believe that golden tests are Wrong"
//
// Fine. Unfortunately, Vitess makes extensive use of golden tests through its test suite, which
// expect byte-wise comparison to be stable between test runs. Using the new version of the
// package would require us to rewrite hundreds of tests, or alternatively, we could disable
// the randomness and call it a day. The method required to disable the randomness is not public, but
// that won't stop us because we're good at computers.
//
// Tracking issue: https://github.com/golang/protobuf/issues/1121
//
//go:linkname DisableProtoBufRandomness google.golang.org/protobuf/internal/detrand.Disable

Of course, this is actually a hack. The ideal solution here would be making sure that no tests in our test suite are actually comparing bytes directly.

There are two ways to get there:

  1. Adding helpers to the test suite that parse the contents of the serialized PB and then comparing the parsed structures directly, instead of comparing the bytes.
  2. Normalizing the contents of the serialized PB (i.e. by removing all whitespace) before we do the literal comparison.

The ideal approach will probably change for each test. There are many tests to fix, but fortunately this is something that can be done incrementally. This is a tracking issue so we can get there.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions