sql: adds unit tests for schemaexpr.CheckConstraintBuilder #49777
Conversation
|
This change is |
| // TODO(mgartner); | ||
| } | ||
|
|
||
| func TestCheckConstraintBuilder_Build(t *testing.T) { |
There was a problem hiding this comment.
[nit]: Do we usually mix camel and snake case?
There was a problem hiding this comment.
Haha good question. I have seen it elsewhere, and Goland suggested it. It looks not very common in cockroachdb, but there are a handful of examples, such as
| } | ||
| } | ||
|
|
||
| func TestCheckConstraintBuilder_DefaultName(t *testing.T) { |
There was a problem hiding this comment.
Do you think this test is somewhat brittle? Or we aren't changing how we generate names for default columns in the future
There was a problem hiding this comment.
In my mind, a "brittle" test is one that breaks when the implementation details change but the API "contract" (in this case input/output) does not. I don't think this is brittle because it is guaranteeing a specific output based on a specific input, but doesn't assert how that output is generated.
If we did want to change this "contract" in the future (AFAIK we have no plans to) then updating the tests to guarantee the new contract seems like a valuable step to me.
There was a problem hiding this comment.
I see -- I wasn't sure there was a set contract on how the constraint names will be formatted.
mgartner
force-pushed
the
unit-test-schema-expr-build
branch
from
aee60f9
to
cc71a6b
Compare
|
LGTM |
This commit adds unit tests to the previously untested functions of `schemaexpra.CheckConstraintBuilder.DefaultName`. It also adds a `testutils.go` file to the `schemaexpr` package with a function that makes it easier to create table descriptors that can be used in tests. Release note: None
mgartner
force-pushed
the
unit-test-schema-expr-build
branch
from
cc71a6b
to
516d593
Compare
|
bors r+ |
Build succeeded |
This commit adds unit tests to the previously untested functions
of
schemaexpra.CheckConstraintBuilder.DefaultName. It also adds atestutils.gofile to theschemaexprpackage with a function thatmakes it easier to create table descriptors that can be used in tests.
Release note: None