Adding upgrade test for TestCustomGetQuery() in graphql/e2e/custom_logic #8848
Conversation
| } | ||
| } | ||
|
|
||
| func Equal(expected interface{}, actual interface{}) bool { |
There was a problem hiding this comment.
Does require.Equal not work?
| return reflect.DeepEqual(expected, actual) | ||
| } | ||
|
|
||
| func (hc *HTTPClient) SafelyUpdateGQLSchema(schema string, alphaPort string) *GqlSchema { |
There was a problem hiding this comment.
We already have UpdateGQLSchema function that is already available in dgraphtest package?
There was a problem hiding this comment.
I still see this function here
dgraphtest/graphql.go
Outdated
| return nil | ||
| } | ||
|
|
||
| func (hc *HTTPClient) RetryProbeGraphQL(alphaPort string) *ProbeGraphQLResp { |
There was a problem hiding this comment.
we probably do not need this function. This could be replaced retryProbeGraphQL function
dgraphtest/graphql.go
Outdated
| } | ||
| } | ||
| if err := json.Unmarshal(updateResp, &updateResult); err != nil { | ||
| debug.PrintStack() |
There was a problem hiding this comment.
when we panic, we do not need to call debug.PrintStack()
| RequireNoError(err) | ||
| // require.Equalf(t, updateResult.UpdateGQLSchema.GqlSchema.Schema, schema, | ||
| // "updateGQLSchema response doesn't reflect the updated schema") | ||
| if !Equal(updateResult.UpdateGQLSchema.GqlSchema.Schema, schema) { |
There was a problem hiding this comment.
why are we not using require.NotEqual?
dgraphtest/graphql.go
Outdated
| } | ||
| } | ||
|
|
||
| func (hc *HTTPClient) updateGQLSchema(alphaPort string, schema string) ([]byte, error) { |
There was a problem hiding this comment.
this function is already present.
| expected := `{"myFavoriteMovies":[{"id":"0x3","name":"Star Wars","director":[{"id":"0x4","name":"George Lucas"}]},{"id":"0x5","name":"Star Trek","director":[{"id":"0x6","name":"J.J. Abrams"}]}]}` | ||
| require.JSONEq(t, expected, string(result.Data)) | ||
| } | ||
| // func TestCustomGetQuery(t *testing.T) { |
There was a problem hiding this comment.
remove these comments so that the diff looks better.
SiddeshUbale
force-pushed
the
siddesh/graphql_upgrade_test
branch
from
d0ea3c1
to
1296132
Compare
|
|
||
| "github.com/dgraph-io/dgraph/dql" | ||
| "github.com/dgraph-io/dgraph/x" | ||
| "github.com/pkg/errors" |
SiddeshUbale
force-pushed
the
siddesh/graphql_upgrade_test
branch
from
fda0040
to
bf0aeee
Compare
| @@ -37,17 +37,20 @@ const ( | |||
| alphaNameFmt = "%v_alpha%d" | |||
| alphaLNameFmt = "alpha%d" | |||
| volNameFmt = "%v_%v" | |||
| genNameFmt = "%v_mock%d" | |||
There was a problem hiding this comment.
name should come as part of the config
|
|
||
| zeroGrpcPort = "5080" | ||
| zeroHttpPort = "6080" | ||
| alphaInterPort = "7080" | ||
| alphaHttpPort = "8080" | ||
| alphaGrpcPort = "9080" | ||
| genericPort = "8888" |
There was a problem hiding this comment.
port should come as part of the config too because any other container could expose a different port
| } | ||
|
|
||
| func (gen *genericContainer) cmd(c *LocalCluster) []string { | ||
| zcmd := []string{} |
There was a problem hiding this comment.
could just do return []string{}
| DefaultExportDir = "/data/exports" | ||
| alphaWorkingDir = "/data/alpha" | ||
| zeroWorkingDir = "/data/zero" | ||
| genericWorkingDir = "/go/src/cmd" |
There was a problem hiding this comment.
this also has to come as part of the config, could be different for different containers.
| type ClusterConfig struct { | ||
| prefix string | ||
| numAlphas int | ||
| numZeros int | ||
| genContainers int |
There was a problem hiding this comment.
nit: genCs is also a good name
| } | ||
|
|
||
| func (gen *genericContainer) healthURL(c *LocalCluster) (string, error) { | ||
| publicPort, err := publicPort(c.dcli, gen, genericPort) |
There was a problem hiding this comment.
health URL won't exist for genC
| return "http://localhost:" + publicPort + "/health", nil | ||
| } | ||
|
|
||
| func (gen *genericContainer) assignURL(c *LocalCluster) (string, error) { |
| Extensions map[string]interface{} `json:"extensions,omitempty"` | ||
| } | ||
|
|
||
| var safelyUpdateGQLSchemaErr = "New Counter: %v, Old Counter: %v.\n" + |
| " retries. The most probable cause is the new GraphQL schema is same as the old" + | ||
| " GraphQL schema." | ||
|
|
||
| func RequireNoError(err error) { |
There was a problem hiding this comment.
we already have x.Panic and other functions, we don't need this
| return reflect.DeepEqual(expected, actual) | ||
| } | ||
|
|
||
| func (hc *HTTPClient) SafelyUpdateGQLSchema(schema string, alphaPort string) *GqlSchema { |
There was a problem hiding this comment.
I still see this function here
Adding upgrade test for TestCustomGetQuery() in graphql/e2e/custom_logic.
Problem:-
1] Added test only for TestCustomGetQuery() function from graphql/e2e/custom_logic.
2] Comments and commented code lines are not yet removed.
3] Some logs which are useful for development are not yet removed.