From 829df031c351dda60302945c39c5e1309075a485 Mon Sep 17 00:00:00 2001
From: Narendra Pathai <narendra.pathai@gmail.com>
Date: Mon, 15 Oct 2018 16:36:27 +0530
Subject: [PATCH] Intermittent failure was due to Thread.sleep in the code.
 While performing unit test cases there was race condition between two
 threads, so it was not guaranteed to work every time. Used an interface
 DelayProvider for simulating delay, and while unit testing fake delay
 provider is used that eradicates the use of Threads in unit test cases, which
 is not a good practice.

---
 .../com/iluwatar/balking/DelayProvider.java   | 10 +++++
 .../com/iluwatar/balking/WashingMachine.java  | 33 ++++++++++----
 .../iluwatar/balking/WashingMachineTest.java  | 44 ++++++++++---------
 3 files changed, 59 insertions(+), 28 deletions(-)
 create mode 100644 balking/src/main/java/com/iluwatar/balking/DelayProvider.java

diff --git a/balking/src/main/java/com/iluwatar/balking/DelayProvider.java b/balking/src/main/java/com/iluwatar/balking/DelayProvider.java
new file mode 100644
index 000000000..26b4f8936
--- /dev/null
+++ b/balking/src/main/java/com/iluwatar/balking/DelayProvider.java
@@ -0,0 +1,10 @@
+package com.iluwatar.balking;
+
+import java.util.concurrent.TimeUnit;
+
+/**
+ * An interface to simulate delay while executing some work.
+ */
+public interface DelayProvider {
+  void executeAfterDelay(long interval, TimeUnit timeUnit, Runnable task);
+}
diff --git a/balking/src/main/java/com/iluwatar/balking/WashingMachine.java b/balking/src/main/java/com/iluwatar/balking/WashingMachine.java
index e4f1259ad..5aa39c66c 100644
--- a/balking/src/main/java/com/iluwatar/balking/WashingMachine.java
+++ b/balking/src/main/java/com/iluwatar/balking/WashingMachine.java
@@ -25,17 +25,38 @@ package com.iluwatar.balking;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import java.util.concurrent.TimeUnit;
+
 /**
  * Washing machine class
  */
 public class WashingMachine {
 
   private static final Logger LOGGER = LoggerFactory.getLogger(WashingMachine.class);
-
+  private final DelayProvider delayProvider;
   private WashingMachineState washingMachineState;
 
+  /**
+   * Creates a new instance of WashingMachine
+   */
   public WashingMachine() {
-    washingMachineState = WashingMachineState.ENABLED;
+    this((interval, timeUnit, task) -> {
+      try {
+        Thread.sleep(timeUnit.toMillis(interval));
+      } catch (InterruptedException ie) {
+        ie.printStackTrace();
+      }
+      task.run();
+    });
+  }
+
+  /**
+   * Creates a new instance of WashingMachine using provided delayProvider. This constructor is used only for
+   * unit testing purposes.
+   */
+  public WashingMachine(DelayProvider delayProvider) {
+    this.delayProvider = delayProvider;
+    this.washingMachineState = WashingMachineState.ENABLED;
   }
 
   public WashingMachineState getWashingMachineState() {
@@ -56,12 +77,8 @@ public class WashingMachine {
       washingMachineState = WashingMachineState.WASHING;
     }
     LOGGER.info("{}: Doing the washing", Thread.currentThread().getName());
-    try {
-      Thread.sleep(50);
-    } catch (InterruptedException ie) {
-      ie.printStackTrace();
-    }
-    endOfWashing();
+
+    this.delayProvider.executeAfterDelay(50, TimeUnit.MILLISECONDS, this::endOfWashing);
   }
 
   /**
diff --git a/balking/src/test/java/com/iluwatar/balking/WashingMachineTest.java b/balking/src/test/java/com/iluwatar/balking/WashingMachineTest.java
index 9a13b8b2d..100bf61a6 100644
--- a/balking/src/test/java/com/iluwatar/balking/WashingMachineTest.java
+++ b/balking/src/test/java/com/iluwatar/balking/WashingMachineTest.java
@@ -22,11 +22,8 @@
  */
 package com.iluwatar.balking;
 
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -36,32 +33,39 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
  */
 public class WashingMachineTest {
 
-  private volatile WashingMachineState machineStateGlobal;
+  private FakeDelayProvider fakeDelayProvider = new FakeDelayProvider();
 
-  @Disabled
   @Test
-  public void wash() throws Exception {
-    WashingMachine washingMachine = new WashingMachine();
-    ExecutorService executorService = Executors.newFixedThreadPool(2);
-    executorService.execute(washingMachine::wash);
-    executorService.execute(() -> {
-      washingMachine.wash();
-      machineStateGlobal = washingMachine.getWashingMachineState();
-    });
-    executorService.shutdown();
-    try {
-      executorService.awaitTermination(10, TimeUnit.SECONDS);
-    } catch (InterruptedException ie) {
-      ie.printStackTrace();
-    }
+  public void wash() {
+    WashingMachine washingMachine = new WashingMachine(fakeDelayProvider);
+
+    washingMachine.wash();
+    washingMachine.wash();
+
+    WashingMachineState machineStateGlobal = washingMachine.getWashingMachineState();
+
+    fakeDelayProvider.task.run();
+
+    // washing machine remains in washing state
     assertEquals(WashingMachineState.WASHING, machineStateGlobal);
+
+    // washing machine goes back to enabled state
+    assertEquals(WashingMachineState.ENABLED, washingMachine.getWashingMachineState());
   }
 
   @Test
-  public void endOfWashing() throws Exception {
+  public void endOfWashing() {
     WashingMachine washingMachine = new WashingMachine();
     washingMachine.wash();
     assertEquals(WashingMachineState.ENABLED, washingMachine.getWashingMachineState());
   }
 
+  private class FakeDelayProvider implements DelayProvider {
+    private Runnable task;
+
+    @Override
+    public void executeAfterDelay(long interval, TimeUnit timeUnit, Runnable task) {
+      this.task = task;
+    }
+  }
 }
\ No newline at end of file