Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sql: move end-to-end and KV tests and common test code to a new package sql/tests #20117

Merged
merged 1 commit into from Nov 20, 2017

Conversation

knz
Copy link
Contributor

@knz knz commented Nov 17, 2017

Visible changes here:

  • common test code in sql/tests
  • high-level SQL tests in sql/tests/highlevel
  • SQL/KV tests in sql/tests/sqlkv

@knz knz requested review from jordanlewis and a team November 17, 2017 12:50
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@knz knz requested a review from a team November 17, 2017 13:31
@knz knz force-pushed the 20171117-sql-tests branch 2 times, most recently from 46e1f14 to 67903a7 Compare November 17, 2017 13:34
@knz knz changed the title sql: move high-level SQL tests to a new sql/tests package sql: move high-level and KV tests to new packages in sql/tests Nov 17, 2017
@knz knz force-pushed the 20171117-sql-tests branch 3 times, most recently from 07d3517 to 25f036a Compare November 20, 2017 15:01
@knz
Copy link
Contributor Author

knz commented Nov 20, 2017

Rebased after merge of #20106. PTAL.

@jordanlewis
Copy link
Member

re: highlevel - is there a reason why these tests can't just live in tests? It's already clear to me from the package structure that things inside of ./pkg/sql/tests are "high level" sql tests.

Same question for sqlkv. Why not just put them in tests?


Reviewed 14 of 15 files at r1, 25 of 27 files at r2, 10 of 10 files at r3, 12 of 12 files at r4.
Review status: all files reviewed at latest revision, 2 unresolved discussions, all commit checks successful.


pkg/sql/tests/sqlkv/benchmark_test.go, line 13 at r4 (raw file):

// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
// implied. See the License for the specific language governing
// permissions and limitations under the License.

All of our benchmark tests files are called bench_test.go except for this one.


pkg/sql/tests/sqlkv/main_test.go, line 1 at r4 (raw file):

// Copyright 2015 The Cockroach Authors.

2017?


Comments from Reviewable

@knz
Copy link
Contributor Author

knz commented Nov 20, 2017

Merged the commits and put the files in a single package sql/tests, as we discussed. PTAL.

@knz
Copy link
Contributor Author

knz commented Nov 20, 2017

Review status: 27 of 36 files reviewed at latest revision, 2 unresolved discussions.


pkg/sql/tests/kv_test.go, line 13 at r4 (raw file):

Previously, jordanlewis (Jordan Lewis) wrote…

All of our benchmark tests files are called bench_test.go except for this one.

Reverted.


pkg/sql/tests/sqlkv/main_test.go, line 1 at r4 (raw file):

Previously, jordanlewis (Jordan Lewis) wrote…

2017?

Done.


Comments from Reviewable

@knz knz changed the title sql: move high-level and KV tests to new packages in sql/tests sql: move end-to-end and KV tests and common test code to a new package sql/tests Nov 20, 2017
@jordanlewis
Copy link
Member

:lgtm:


Reviewed 4 of 15 files at r3, 4 of 16 files at r4, 19 of 19 files at r5.
Review status: :shipit: all files reviewed at latest revision, all discussions resolved, all commit checks successful.


Comments from Reviewable

@knz
Copy link
Contributor Author

knz commented Nov 20, 2017

TFYR!

@knz knz merged commit 5528d24 into cockroachdb:master Nov 20, 2017
@knz knz deleted the 20171117-sql-tests branch November 20, 2017 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants