sql: move copy_in_test to pkg/sql/copy to have all copy tests together #96232
Conversation
|
This change is |
cucaroach
force-pushed
the
copy_in_move
branch
from
183fd35 to
65bd41d
Compare
pkg/sql/copy/copy_in_test.go
Outdated
| @@ -8,7 +8,7 @@ | |||
| // by the Apache License, Version 2.0, included in the file | |||
| // licenses/APL.txt. | |||
|
|
|||
| package sql_test | |||
| package copy_test | |||
There was a problem hiding this comment.
this can just be package copy now right
There was a problem hiding this comment.
Umm, IDK, what's the motivation? Is the idea to use _test only when its necessary to break a package dep cycle? I thought _test was good hygiene for the only test/use the public bits perspective (which I don't really buy but not the hill I want to die on). Curious your thoughts...
There was a problem hiding this comment.
_test only when its necessary to break a package dep cycle
that was my understanding. that and it has caused me headaches in the past when doing build work :P but not important here.
I thought _test was good hygiene for the only test/use the public bits perspective
eh, usually for tests you want to do unit tests which involve private methods right...
either way, not a hill i'm looking to die on either.
There was a problem hiding this comment.
I'm gonna go with if you remove _test and everything still works then it shouldn't be there as a good principle here. Thanks!
| @@ -140,3 +142,13 @@ func ArrayStringToSlice(t *testing.T, array string, message ...string) []string | |||
| } | |||
| return strings.Split(array[1:length-1], ",") | |||
| } | |||
|
|
|||
| // PgxConn is a helper to create a pgx.Conn from a url. | |||
| func PgxConn(t *testing.T, connURL url.URL) (*pgx.Conn, error) { | |||
There was a problem hiding this comment.
nit: PGXConn is the best practice form
cucaroach
force-pushed
the
copy_in_move
branch
from
65bd41d to
897d0d4
Compare
Epic: CRDB-18892 Informs: cockroachdb#91831 Release note: None
cucaroach
force-pushed
the
copy_in_move
branch
from
897d0d4 to
9894417
Compare
|
TFTRs! |
|
Build succeeded: |
Epic: CRDB-18892
Informs: #91831
Release note: None