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

kv rest api: add remaining range tests #111

Merged
merged 1 commit into from
Oct 10, 2014
Merged

Conversation

andybons
Copy link
Contributor

@andybons andybons commented Oct 7, 2014

No description provided.

@andybons
Copy link
Contributor Author

andybons commented Oct 7, 2014

@cockroachdb/developers

// Query remaining range.
start, end = 0, 99
Copy link
Member

Choose a reason for hiding this comment

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

could add another limit here (say 75) which shouldn't interfere with the operation.

@tbg
Copy link
Member

tbg commented Oct 8, 2014

LGTM
Squashing would make sense before the merge.

t.Errorf("unable to decode JSON into proto.ScanResponse: %v", err)
}
numRows := end - limit
if len(scan.Rows) != numRows {
Copy link
Member

Choose a reason for hiding this comment

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

Another way to verify would be to construct expected results. You might do that via a Scan of the underlying rows directly from the server's db. That way you could set up a list of tests which scan with different start, end and limit keys and then compare the results using !reflect.DeepEqual(results, expResults).

Not necessary to do this--I think these tests are fine. But something to keep in mind.

@spencerkimball
Copy link
Member

LGTM

@andybons andybons force-pushed the andybons/kvresttests branch 3 times, most recently from 5c8cddc to 069fa16 Compare October 10, 2014 17:26
@andybons andybons force-pushed the andybons/kvresttests branch from 069fa16 to 079034c Compare October 10, 2014 17:28
andybons added a commit that referenced this pull request Oct 10, 2014
kv rest api: add remaining range tests
@andybons andybons merged commit 19b1ac3 into master Oct 10, 2014
@andybons andybons deleted the andybons/kvresttests branch October 10, 2014 17:48
soniabhishek pushed a commit to soniabhishek/cockroach that referenced this pull request Feb 15, 2017
…ive_contains_route

Case sensitive contained in logic
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.

3 participants