*: break circle dependency (#265)

Cargo is not good at dealing with features with circle dependency, see
rust-lang/cargo#4866. In the past, raft depends on harness due to tests,
and harness depends on raft as it's only a wrapper of raft.  This PR
breaks the circle dependencies by moving all the tests from raft to
harness, so only harness needs to depend on raft.

.travis.yml

@@ -33,9 +33,9 @@ script:
   - if [[ $TRAVIS_RUST_VERSION == "stable" && $TRAVIS_OS_NAME == "linux" ]]; then cargo clippy -- -D clippy::all; fi
   - if [[ $TRAVIS_RUST_VERSION == "stable" && $TRAVIS_OS_NAME == "linux" ]]; then cargo clippy --no-default-features --features prost-codec -- -D clippy::all; fi
   - cargo test --all -- --nocapture
-  # There is a bug in protobuf-build that cached size is not supported yet.
-  # TODO: - cargo test --no-default-features --features prost-codec -- --nocapture
   # Validate benches still work.
   - cargo bench --all -- --test
   # Because failpoints inject failure in code path, which will affect all concurrently running tests, Hence they need to be synchronized, which make tests slow.
   - cargo test --tests --features failpoints -- --nocapture
+  # TODO: There is a bug in protobuf-build that cached size is not supported yet, so do not test harness.
+  - cargo test --no-default-features --features prost-codec -- --nocapture

Cargo.toml

@@ -13,22 +13,19 @@ categories = ["algorithms", "database-implementations"]
 edition = "2018"
 
 [workspace]
-members = ["proto"]
+members = ["proto", "harness"]
 
 [features]
 default = ["protobuf-codec"]
 # Enable failpoints
 failpoints = ["fail/failpoints"]
-protobuf-codec = ["raft-proto/protobuf-codec", "harness/protobuf-codec"]
-prost-codec = ["prost-derive", "bytes", "prost", "raft-proto/prost-codec", "harness/prost-codec"]
+protobuf-codec = ["raft-proto/protobuf-codec"]
+prost-codec = ["raft-proto/prost-codec"]
 
 # Make sure to synchronize updates with Harness.
 [dependencies]
 log = ">0.2"
 protobuf = "2"
-prost = { version = "0.5", optional = true }
-prost-derive = { version = "0.5", optional = true }
-bytes = { version = "0.4.11", optional = true }
 slog = "2.2"
 quick-error = "1.2.2"
 raft-proto = { path = "proto", default-features = false }
@@ -37,15 +34,13 @@ hashbrown = "0.5"
 fail = { version = "0.3", optional = true }
 getset = "0.0.7"
 slog-stdlog = "3.0.2"
-slog-term = "2.4.0"
 slog-envlogger = "2.1.0"
 
 [dev-dependencies]
 criterion = ">0.2.4"
-lazy_static = "1.0"
-harness = { path = "harness", default-features = false }
 regex = "1.1"
 slog-async = "2.3.0"
+slog-term = "2.4.0"
 
 [[bench]]
 name = "benches"

harness/Cargo.toml

@@ -23,3 +23,11 @@ rand = "0.7.0"
 slog = "2.2"
 slog-term = "2.4.0"
 slog-envlogger = "2.1.0"
+
+[dev-dependencies]
+criterion = ">0.2.4"
+hashbrown = "0.5"
+lazy_static = "1.0"
+protobuf = "2"
+regex = "1.1"
+slog-async = "2.3.0"

src/log_unstable.rs

@@ -180,9 +180,9 @@ impl Unstable {
 
 #[cfg(test)]
 mod test {
+    use crate::default_logger;
     use crate::eraftpb::{Entry, Snapshot, SnapshotMetadata};
     use crate::log_unstable::Unstable;
-    use harness::testing_logger;
 
     fn new_entry(index: u64, term: u64) -> Entry {
         let mut e = Entry::default();
@@ -202,7 +202,7 @@ mod test {
 
     #[test]
     fn test_maybe_first_index() {
-        testing_logger().new(o!("test" => "maybe_first_index"));
+        default_logger().new(o!("test" => "maybe_first_index"));
         // entry, offset, snap, wok, windex,
         let tests = vec![
             // no snapshot
@@ -230,7 +230,7 @@ mod test {
 
     #[test]
     fn test_maybe_last_index() {
-        testing_logger().new(o!("test" => "maybe_last_index"));
+        default_logger().new(o!("test" => "maybe_last_index"));
         // entry, offset, snap, wok, windex,
         let tests = vec![
             (Some(new_entry(5, 1)), 5, None, true, 5),
@@ -258,7 +258,7 @@ mod test {
 
     #[test]
     fn test_maybe_term() {
-        testing_logger().new(o!("test" => "maybe_term"));
+        default_logger().new(o!("test" => "maybe_term"));
         // entry, offset, snap, index, wok, wterm
         let tests = vec![
             // term from entries
@@ -320,7 +320,7 @@ mod test {
 
     #[test]
     fn test_restore() {
-        testing_logger().new(o!("test" => "restore"));
+        default_logger().new(o!("test" => "restore"));
         let mut u = Unstable {
             entries: vec![new_entry(5, 1)],
             offset: 5,
@@ -338,7 +338,7 @@ mod test {
 
     #[test]
     fn test_stable_to() {
-        testing_logger().new(o!("test" => "stable_to"));
+        default_logger().new(o!("test" => "stable_to"));
         // entries, offset, snap, index, term, woffset, wlen
         let tests = vec![
             (vec![], 0, None, 5, 1, 0, 0),
@@ -418,7 +418,7 @@ mod test {
 
     #[test]
     fn test_truncate_and_append() {
-        testing_logger().new(o!("test" => "truncate_and_append"));
+        default_logger().new(o!("test" => "truncate_and_append"));
         // entries, offset, snap, to_append, woffset, wentries
         let tests = vec![
             // replace to the end

src/progress/inflights.rs

@@ -138,11 +138,11 @@ impl Inflights {
 #[cfg(test)]
 mod tests {
     use super::Inflights;
-    use harness::testing_logger;
+    use crate::default_logger;
 
     #[test]
     fn test_inflight_add() {
-        let _ = testing_logger().new(o!("test" => "inflight_add"));
+        let _ = default_logger().new(o!("test" => "inflight_add"));
         let mut inflight = Inflights::new(10);
         for i in 0..5 {
             inflight.add(i);
@@ -199,7 +199,7 @@ mod tests {
 
     #[test]
     fn test_inflight_free_to() {
-        let _ = testing_logger().new(o!("test" => "inflight_free_to"));
+        let _ = default_logger().new(o!("test" => "inflight_free_to"));
         let mut inflight = Inflights::new(10);
         for i in 0..10 {
             inflight.add(i);
@@ -252,7 +252,7 @@ mod tests {
 
     #[test]
     fn test_inflight_free_first_one() {
-        let _ = testing_logger().new(o!("test" => "inflight_free_first_one"));
+        let _ = default_logger().new(o!("test" => "inflight_free_first_one"));
         let mut inflight = Inflights::new(10);
         for i in 0..10 {
             inflight.add(i);

src/progress/progress_set.rs

@@ -112,13 +112,14 @@ impl Configuration {
     /// * There must be at least one voter.
     pub fn valid(&self) -> Result<()> {
         if let Some(id) = self.voters.intersection(&self.learners).next() {
-            Err(Error::Exists(*id, "learners"))?;
+            Err(Error::Exists(*id, "learners"))
         } else if self.voters.is_empty() {
             Err(Error::ConfigInvalid(
                 "There must be at least one voter.".into(),
-            ))?;
+            ))
+        } else {
+            Ok(())
         }
-        Ok(())
     }
 
     fn has_quorum(&self, potential_quorum: &HashSet<u64>) -> bool {
@@ -650,7 +651,7 @@ impl ProgressSet {
     pub fn finalize_membership_change(&mut self) -> Result<()> {
         let next = self.next_configuration.take();
         match next {
-            None => Err(Error::NoPendingMembershipChange)?,
+            None => Err(Error::NoPendingMembershipChange),
             Some(next) => {
                 {
                     let pending = self
@@ -669,9 +670,9 @@ impl ProgressSet {
                     "Finalizing membership change";
                     "config" => ?self.configuration,
                 );
+                Ok(())
             }
         }
-        Ok(())
     }
 }
 
@@ -680,15 +681,15 @@ impl ProgressSet {
 #[cfg(test)]
 mod test_progress_set {
     use super::{Configuration, ProgressSet, Result};
+    use crate::default_logger;
     use crate::progress::Progress;
-    use harness::testing_logger;
     use hashbrown::HashSet;
 
     const CANARY: u64 = 123;
 
     #[test]
     fn test_insert_redundant_voter() -> Result<()> {
-        let l = testing_logger().new(o!("test" => "test_insert_redundant_voter"));
+        let l = default_logger().new(o!("test" => "test_insert_redundant_voter"));
         let mut set = ProgressSet::new().with_logger(&l);
         let default_progress = Progress::new(0, 256);
         let mut canary_progress = Progress::new(0, 256);
@@ -708,7 +709,7 @@ mod test_progress_set {
 
     #[test]
     fn test_insert_redundant_learner() -> Result<()> {
-        let l = testing_logger().new(o!("test" => "test_insert_redundant_learner"));
+        let l = default_logger().new(o!("test" => "test_insert_redundant_learner"));
         let mut set = ProgressSet::new().with_logger(&l);
         let default_progress = Progress::new(0, 256);
         let mut canary_progress = Progress::new(0, 256);
@@ -728,7 +729,7 @@ mod test_progress_set {
 
     #[test]
     fn test_insert_learner_that_is_voter() -> Result<()> {
-        let l = testing_logger().new(o!("test" => "test_insert_learner_that_is_voter"));
+        let l = default_logger().new(o!("test" => "test_insert_learner_that_is_voter"));
         let mut set = ProgressSet::new().with_logger(&l);
         let default_progress = Progress::new(0, 256);
         let mut canary_progress = Progress::new(0, 256);
@@ -748,7 +749,7 @@ mod test_progress_set {
 
     #[test]
     fn test_insert_voter_that_is_learner() -> Result<()> {
-        let l = testing_logger().new(o!("test" => "test_insert_voter_that_is_learner"));
+        let l = default_logger().new(o!("test" => "test_insert_voter_that_is_learner"));
         let mut set = ProgressSet::new().with_logger(&l);
         let default_progress = Progress::new(0, 256);
         let mut canary_progress = Progress::new(0, 256);
@@ -768,7 +769,7 @@ mod test_progress_set {
 
     #[test]
     fn test_promote_learner() -> Result<()> {
-        let l = testing_logger().new(o!("test" => "test_promote_learner"));
+        let l = default_logger().new(o!("test" => "test_promote_learner"));
         let mut set = ProgressSet::new().with_logger(&l);
         let default_progress = Progress::new(0, 256);
         set.insert_voter(1, default_progress)?;
@@ -833,7 +834,7 @@ mod test_progress_set {
         start: (impl IntoIterator<Item = u64>, impl IntoIterator<Item = u64>),
         end: (impl IntoIterator<Item = u64>, impl IntoIterator<Item = u64>),
     ) -> Result<()> {
-        let l = testing_logger().new(o!("test" => "check_membership_change_configuration"));
+        let l = default_logger().new(o!("test" => "check_membership_change_configuration"));
         let start_voters = start.0.into_iter().collect::<HashSet<u64>>();
         let start_learners = start.1.into_iter().collect::<HashSet<u64>>();
         let end_voters = end.0.into_iter().collect::<HashSet<u64>>();