Skip to content

cli: automatically detect SQL commands in tests#54932

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
knz:20200929-cli-snag
Sep 30, 2020
Merged

cli: automatically detect SQL commands in tests#54932
craig[bot] merged 1 commit intocockroachdb:masterfrom
knz:20200929-cli-snag

Conversation

@knz
Copy link
Copy Markdown
Contributor

@knz knz commented Sep 29, 2020

Motivated by seeing @spaskob and @adityamaru run into this twice in a month.

Prior to this change, every new SQL-only client command needed to be
recognized by the isSQLCommand function in cli_test.go. This was a
snag because nothing in the "main" code was suggesting this was
necessary, and failure to do so would cause extremely obscure EOF
connection errors during tests.

This patch improves the situation by automatically recognizing
the SQL commands in tests.

Release note: None

Prior to this change, every new SQL-only client command needed to be
recognized by the `isSQLCommand` function in `cli_test.go`. This was a
snag because nothing in the "main" code was suggesting this was
necessary, and failure to do so would cause extremely obscure EOF
connection errors during tests.

This patch improves the situation by automatically recognizing
the SQL commands in tests.

Release note: None
@knz knz requested review from irfansharif and tbg September 29, 2020 10:38
@knz knz requested a review from a team as a code owner September 29, 2020 10:38
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@knz
Copy link
Copy Markdown
Contributor Author

knz commented Sep 30, 2020

TFYR

bors=tbg,irfansharif

@knz
Copy link
Copy Markdown
Contributor Author

knz commented Sep 30, 2020

bors r=tbg,irfansharif

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Sep 30, 2020

Build succeeded:

@craig craig bot merged commit 361acae into cockroachdb:master Sep 30, 2020
@knz knz deleted the 20200929-cli-snag branch September 30, 2020 09:49
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.

4 participants