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

Add few doltpy commands that broke with dolt compatibility changes #1479

Merged
merged 3 commits into from Mar 25, 2021

Conversation

max-hoffman
Copy link
Contributor

Some bats that if failed indicate a doltpy incompatibility. The tests could be more involved, but the goal here isn't row-output correctness.

This list will grow as I rewrite more dolt commands to use SQL features.

Copy link
Sponsor Contributor

@timsehn timsehn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple comments.

# "SELECT * FROM `{table}` ASOF '{commit}'"

# verifying the output comparison to dolt log would be nice
@test "doltpy: Dolt.log" {
Copy link
Sponsor Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should put the assertion in the test name. Like:

"dolt log returns proper named headers"

ORDER BY date DESC
LIMIT 1;
SQL
echo $status
Copy link
Sponsor Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Get rid of theses. They are trapped if the test passes and you get output if it fails. echos are useless.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixing

@timsehn
Copy link
Sponsor Contributor

timsehn commented Mar 25, 2021

LGTM

@max-hoffman max-hoffman merged commit 4703f6d into master Mar 25, 2021
@max-hoffman max-hoffman deleted the max/doltpy-bats branch March 25, 2021 23:56
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

2 participants