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 proptest support #68

Merged
merged 9 commits into from
Jul 5, 2019
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ edition = "2018"
default = []
# Enable integration tests with a running TiKV and PD instance.
# Use $PD_ADDRS, comma separated, to set the addresses the tests use.
integration-tests = []
integration-tests = ["property-testing"]
# Enable propery testing features with `proptest`.
property-testing = ["proptest", "proptest-derive"]


[lib]
name = "tikv_client"
Expand All @@ -28,6 +31,8 @@ serde = "1.0"
serde_derive = "1.0"
tokio-core = "0.1"
tokio-timer = "0.2"
proptest = { version = "0.9", optional = true }
proptest-derive = { version = "0.1.0", optional = true }

[dependencies.kvproto]
git = "https://github.com/pingcap/kvproto.git"
Expand Down
28 changes: 26 additions & 2 deletions src/kv.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,19 @@
// Copyright 2019 TiKV Project Authors. Licensed under Apache-2.0.

#[cfg(feature = "proptest-derive")]
Hoverbear marked this conversation as resolved.
Show resolved Hide resolved
use proptest::{arbitrary::any_with, collection::size_range};
#[cfg(feature = "proptest-derive")]
use proptest_derive::Arbitrary;
use std::ops::{
Bound, Deref, DerefMut, Range, RangeFrom, RangeFull, RangeInclusive, RangeTo, RangeToInclusive,
};
use std::{fmt, str, u8};

#[cfg(feature = "proptest-derive")]
const PROPTEST_KEY_MAX: usize = 1024 * 2; // 2 KB
#[cfg(feature = "proptest-derive")]
const PROPTEST_VALUE_MAX: usize = 1024 * 16; // 16 KB

use crate::{Error, Result};

struct HexRepr<'a>(pub &'a [u8]);
Expand Down Expand Up @@ -56,7 +65,14 @@ impl<'a> fmt::Display for HexRepr<'a> {
/// accept an `Into<Key>`, which means all of the above types can be passed directly to those
/// functions.
#[derive(Default, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)]
pub struct Key(Vec<u8>);
#[cfg_attr(feature = "proptest-derive", derive(Arbitrary))]
pub struct Key(
#[cfg_attr(
feature = "proptest-derive",
proptest(strategy = "any_with::<Vec<u8>>((size_range(PROPTEST_KEY_MAX), ()))")
)]
Vec<u8>,
);

impl Key {
#[inline]
Expand Down Expand Up @@ -170,7 +186,14 @@ impl fmt::Debug for Key {
/// accept an `Into<Value>`, which means all of the above types can be passed directly to those
/// functions.
#[derive(Default, Clone, Eq, PartialEq, Hash)]
pub struct Value(Vec<u8>);
#[cfg_attr(feature = "proptest-derive", derive(Arbitrary))]
pub struct Value(
#[cfg_attr(
feature = "proptest-derive",
proptest(strategy = "any_with::<Vec<u8>>((size_range(PROPTEST_VALUE_MAX), ()))")
)]
Vec<u8>,
);

impl Value {
#[inline]
Expand Down Expand Up @@ -239,6 +262,7 @@ impl fmt::Debug for Value {
/// Many functions which accept a `KvPair` accept an `Into<KvPair>`, which means all of the above
/// types (Like a `(Key, Value)`) can be passed directly to those functions.
#[derive(Default, Clone, Eq, PartialEq)]
#[cfg_attr(feature = "proptest-derive", derive(Arbitrary))]
pub struct KvPair(Key, Value);

impl KvPair {
Expand Down
28 changes: 14 additions & 14 deletions tests/integration_tests/mod.rs → tests/integration/mod.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
// Copyright 2019 TiKV Project Authors. Licensed under Apache-2.0.
mod raw;
use std::env::var;
const ENV_PD_ADDR: &str = "PD_ADDR";
pub fn pd_addr() -> Vec<String> {
var(ENV_PD_ADDR)
.expect(&format!("Expected {}:", ENV_PD_ADDR))
.split(",")
.map(From::from)
.collect()
}
// Copyright 2019 TiKV Project Authors. Licensed under Apache-2.0.

mod raw;

use std::env::var;
const ENV_PD_ADDR: &str = "PD_ADDR";

pub fn pd_addr() -> Vec<String> {
var(ENV_PD_ADDR)
.expect(&format!("Expected {}:", ENV_PD_ADDR))
.split(",")
.map(From::from)
.collect()
}
82 changes: 82 additions & 0 deletions tests/integration/raw.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
// Copyright 2019 TiKV Project Authors. Licensed under Apache-2.0.

use crate::{arb_batch, integration::pd_addr};
use futures::executor::block_on;
use proptest::{arbitrary::any, proptest};
use tikv_client::{raw::Client, Config, KvPair, Value};

proptest! {
Hoverbear marked this conversation as resolved.
Show resolved Hide resolved
#[test]
fn point(
pair in any::<KvPair>(),
) {
let client = block_on(Client::connect(Config::new(pd_addr()))).unwrap();

block_on(
client.put(pair.key().clone(), pair.value().clone())
).unwrap();
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would use await rather than block_on since the latter does not permit concurrency

Copy link
Contributor Author

@Hoverbear Hoverbear Jun 19, 2019

Choose a reason for hiding this comment

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

I would too!

But, the proptest macro isn't compatible with async/await, I tried using a bare approach with async test functions but since the TestRunner operates in a closure (that doesn't return a result or an async) you can't use .await or ? anyways.

I'm considering trying to patch this upstream, but in the meantime, this seems easiest as it avoids a fair bit of testrunner/handling boilerplate.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I was thinking you would just use another function and call that from the test function, but if you think that is not worth the boilerplate, then we can keep things like this


let out_value = block_on(
client.get(pair.key().clone())
).unwrap();
assert_eq!(Some(Value::from(pair.value().clone())), out_value);

block_on(
client.delete(pair.key().clone())
).unwrap();
}
}

proptest! {
#[test]
fn batch(
kvs in arb_batch(any::<KvPair>(), None),
) {
let client = block_on(Client::connect(Config::new(pd_addr()))).unwrap();
let keys = kvs.iter().map(|kv| kv.key()).cloned().collect::<Vec<_>>();

block_on(
client.batch_put(kvs.clone())
).unwrap();

let out_value = block_on(
client.batch_get(keys.clone())
).unwrap();
assert_eq!(kvs, out_value);

block_on(
client.batch_delete(keys.clone())
).unwrap();
}
}

proptest! {
#[test]
fn scan(
kvs in arb_batch(any::<KvPair>(), None),
) {
let client = block_on(Client::connect(Config::new(pd_addr()))).unwrap();
let mut keys = kvs.iter().map(|kv| kv.key()).cloned().collect::<Vec<_>>();
keys.sort();
// If we aren't getting values this is an empty vector, so use dummy values.
let start_key = keys.iter().cloned().next().unwrap_or(vec![0].into());
let end_key = keys.iter().cloned().last().unwrap_or(vec![0].into());

block_on(
client.batch_put(kvs.clone())
).unwrap();

let out_value = block_on(
client.scan(start_key..=end_key, 10240) // Magic max number is TiKV intrinsic
).unwrap();

// Since TiKV returns empty keys as tombstones in scans, we just check we can find all the items.
assert!(kvs.iter().all(|kv| {
out_value.iter().find(|out| kv == *out).is_some()
}));

block_on(
client.batch_delete(keys.clone())
).unwrap();
}
}
161 changes: 0 additions & 161 deletions tests/integration_tests/raw.rs

This file was deleted.

17 changes: 16 additions & 1 deletion tests/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,19 @@
#![feature(async_await)]

#[cfg(feature = "integration-tests")]
mod integration_tests;
mod integration;

#[cfg(feature = "proptest")]
use proptest::strategy::Strategy;

#[cfg(feature = "proptest")]
const PROPTEST_BATCH_SIZE_MAX: usize = 16;

#[cfg(feature = "proptest")]
pub fn arb_batch<T: core::fmt::Debug>(
single_strategy: impl Strategy<Value = T>,
max_batch_size: impl Into<Option<usize>>,
) -> impl Strategy<Value = Vec<T>> {
let max_batch_size = max_batch_size.into().unwrap_or(PROPTEST_BATCH_SIZE_MAX);
proptest::collection::vec(single_strategy, 0..max_batch_size)
}