Merge pull request #19 from d4rken/pr-builderreuse

Fix RxCmdShell.Builder reuse
d4rken · Mar 14, 2018 · 7846361 · 7846361
2 parents 71e509a + da22a65
commit 7846361
Show file tree

Hide file tree

Showing 5 changed files with 30 additions and 48 deletions.
diff --git a/README.md b/README.md
@@ -8,7 +8,7 @@ A library that helps your app interact with shells on Android.
 ## Quickstart
 Include the library in your modules `build.gradle` file:
 ```groovy
-implementation 'eu.darken.rxshell:core:0.9.8'
+implementation 'eu.darken.rxshell:core:0.9.9'
 ```
 
 Now your project is ready to use the library, let's quickly talk about a few core concepts:

diff --git a/core/build.gradle b/core/build.gradle
@@ -8,7 +8,7 @@ apply plugin: 'com.android.library'
 
 def versionMajor = 0
 def versionMinor = 9
-def versionPatch = 8
+def versionPatch = 9
 def myVersionCode = versionMajor * 10000 + versionMinor * 100 + versionPatch
 def myVersionName = "${versionMajor}.${versionMinor}.${versionPatch}"
 

diff --git a/core/src/main/java/eu/darken/rxshell/cmd/RxCmdShell.java b/core/src/main/java/eu/darken/rxshell/cmd/RxCmdShell.java
@@ -40,10 +40,10 @@ private RxCmdShell() throws InstantiationException {
         throw new InstantiationException("Use the builder()!");
     }
 
-    RxCmdShell(Builder builder) {
+    RxCmdShell(Builder builder, RxShell rxShell) {
         environment = builder.getEnvironment();
-        rxShell = builder.getRxShell();
         processorFactory = builder.getProcessorFactory();
+        this.rxShell = rxShell;
     }
 
     /**
@@ -216,16 +216,11 @@ public static class Builder {
         private final Map<String, String> environment = new HashMap<>();
         private final CmdProcessor.Factory processorFactory = new CmdProcessor.Factory(new Harvester.Factory());
         private boolean useRoot = false;
-        private RxShell rxShell;
 
         CmdProcessor.Factory getProcessorFactory() {
             return processorFactory;
         }
 
-        RxShell getRxShell() {
-            return rxShell;
-        }
-
         /**
          * Environment variables that will be set when opening the shell session.
          * <p>
@@ -283,13 +278,13 @@ public RxCmdShell build() {
             for (HasEnvironmentVariables envVars : envVarSources) {
                 shellEnvironment(envVars.getEnvironmentVariables(useRoot));
             }
-            if (rxShell == null) {
-                final ProcessFactory processFactory = new DefaultProcessFactory();
-                final ProcessKiller processKiller = useRoot ? new RootKiller(processFactory) : new UserKiller();
-                final String command = useRoot ? "su" : "sh";
-                rxShell = new RxShell(new RxProcess(processFactory, processKiller, command));
-            }
-            return new RxCmdShell(this);
+
+            final ProcessFactory processFactory = new DefaultProcessFactory();
+            final ProcessKiller processKiller = useRoot ? new RootKiller(processFactory) : new UserKiller();
+            final String command = useRoot ? "su" : "sh";
+            RxShell rxShell = new RxShell(new RxProcess(processFactory, processKiller, command));
+
+            return new RxCmdShell(this, rxShell);
         }
 
         /**

diff --git a/core/src/test/java/eu/darken/rxshell/cmd/RxCmdShellBuilderTest.java b/core/src/test/java/eu/darken/rxshell/cmd/RxCmdShellBuilderTest.java
@@ -27,15 +27,6 @@ public void testInstantiation() {
         assertThat(shell, is(not(nullValue())));
     }
 
-    @Test
-    public void testAutoRxShellCreation() {
-        final RxCmdShell.Builder shellBuilder = RxCmdShell.builder();
-        assertThat(shellBuilder.getRxShell(), is(nullValue()));
-        shellBuilder.build();
-        assertThat(shellBuilder.getRxShell(), is(not(nullValue())));
-    }
-
-
     @Test
     public void testEnvironmentBuilding() {
         RxCmdShell.Builder shellBuilder = RxCmdShell.builder();

diff --git a/core/src/test/java/eu/darken/rxshell/cmd/RxCmdShellTest.java b/core/src/test/java/eu/darken/rxshell/cmd/RxCmdShellTest.java
@@ -44,8 +44,6 @@ public void setup() throws Exception {
         BehaviorSubject<Boolean> idlePub = BehaviorSubject.createDefault(true);
         when(cmdProcessor.isIdle()).thenReturn(idlePub);
 
-        when(builder.getRxShell()).thenReturn(rxShell);
-
         when(rxShell.open()).thenReturn(Single.create(emitter -> {
             when(rxShellSession.waitFor()).thenReturn(Single.create(e -> waitForEmitter = e));
             emitter.onSuccess(rxShellSession);
@@ -71,22 +69,21 @@ public void setup() throws Exception {
 
     @Test
     public void testInstantiation() {
-        new RxCmdShell(builder);
-        verify(builder).getRxShell();
+        new RxCmdShell(builder, rxShell);
         verify(builder).getEnvironment();
     }
 
     @Test
     public void testOpen() {
-        RxCmdShell rxCmdShell = new RxCmdShell(builder);
+        RxCmdShell rxCmdShell = new RxCmdShell(builder, rxShell);
         rxCmdShell.open().test().awaitCount(1).assertNoTimeout();
         verify(rxShell).open();
     }
 
     @Test
     public void testOpen_exception() {
         doReturn(Single.error(new IOException())).when(rxShell).open();
-        RxCmdShell rxCmdShell = new RxCmdShell(builder);
+        RxCmdShell rxCmdShell = new RxCmdShell(builder, rxShell);
         rxCmdShell.open().test().awaitDone(1, TimeUnit.SECONDS).assertNoTimeout().assertError(IOException.class);
         verify(rxShell).open();
 
@@ -96,7 +93,7 @@ public void testOpen_exception() {
 
     @Test
     public void testCancel() {
-        RxCmdShell shell = new RxCmdShell(builder);
+        RxCmdShell shell = new RxCmdShell(builder, rxShell);
         RxCmdShell.Session session1 = shell.open().test().awaitDone(1, TimeUnit.SECONDS).assertNoTimeout().values().get(0);
         session1.isAlive().test().awaitDone(1, TimeUnit.SECONDS).assertNoTimeout().assertValue(true);
 
@@ -110,7 +107,7 @@ public void testCancel() {
 
     @Test
     public void testCancel_indirect() {
-        RxCmdShell shell = new RxCmdShell(builder);
+        RxCmdShell shell = new RxCmdShell(builder, rxShell);
         RxCmdShell.Session session1 = shell.open().test().awaitDone(1, TimeUnit.SECONDS).assertNoTimeout().values().get(0);
         session1.isAlive().test().awaitDone(1, TimeUnit.SECONDS).assertNoTimeout().assertValue(true);
 
@@ -125,7 +122,7 @@ public void testCancel_indirect() {
 
     @Test
     public void testCancel_thenClose() {
-        RxCmdShell shell = new RxCmdShell(builder);
+        RxCmdShell shell = new RxCmdShell(builder, rxShell);
         RxCmdShell.Session session1 = shell.open().test().awaitDone(1, TimeUnit.SECONDS).assertNoTimeout().values().get(0);
         session1.isAlive().test().awaitDone(1, TimeUnit.SECONDS).assertNoTimeout().assertValue(true);
 
@@ -135,7 +132,7 @@ public void testCancel_thenClose() {
 
     @Test
     public void testCancel_thenClose_indirect() {
-        RxCmdShell shell = new RxCmdShell(builder);
+        RxCmdShell shell = new RxCmdShell(builder, rxShell);
         shell.open().test().awaitDone(1, TimeUnit.SECONDS).assertNoTimeout().values().get(0);
         shell.isAlive().test().awaitDone(1, TimeUnit.SECONDS).assertNoTimeout().assertValue(true);
 
@@ -145,7 +142,7 @@ public void testCancel_thenClose_indirect() {
 
     @Test
     public void testClose() throws IOException {
-        RxCmdShell shell = new RxCmdShell(builder);
+        RxCmdShell shell = new RxCmdShell(builder, rxShell);
         RxCmdShell.Session session1 = shell.open().test().awaitDone(1, TimeUnit.SECONDS).assertNoTimeout().values().get(0);
         session1.isAlive().test().awaitDone(1, TimeUnit.SECONDS).assertNoTimeout().assertValue(true);
 
@@ -159,7 +156,7 @@ public void testClose() throws IOException {
 
     @Test
     public void testClose_indirect() {
-        RxCmdShell shell = new RxCmdShell(builder);
+        RxCmdShell shell = new RxCmdShell(builder, rxShell);
         RxCmdShell.Session session1 = shell.open().test().awaitDone(1, TimeUnit.SECONDS).assertNoTimeout().values().get(0);
         session1.isAlive().test().awaitDone(1, TimeUnit.SECONDS).assertNoTimeout().assertValue(true);
 
@@ -173,7 +170,7 @@ public void testClose_indirect() {
 
     @Test
     public void testClose_thenCancel() {
-        RxCmdShell shell = new RxCmdShell(builder);
+        RxCmdShell shell = new RxCmdShell(builder, rxShell);
         RxCmdShell.Session session1 = shell.open().test().awaitDone(1, TimeUnit.SECONDS).assertNoTimeout().values().get(0);
         session1.isAlive().test().awaitDone(1, TimeUnit.SECONDS).assertNoTimeout().assertValue(true);
 
@@ -183,7 +180,7 @@ public void testClose_thenCancel() {
 
     @Test
     public void testClose_thenCancel_indirect() {
-        RxCmdShell shell = new RxCmdShell(builder);
+        RxCmdShell shell = new RxCmdShell(builder, rxShell);
         shell.open().test().awaitDone(1, TimeUnit.SECONDS).assertNoTimeout().values().get(0);
         shell.isAlive().test().awaitDone(1, TimeUnit.SECONDS).assertNoTimeout().assertValue(true);
 
@@ -193,7 +190,7 @@ public void testClose_thenCancel_indirect() {
 
     @Test
     public void testAlive() throws IOException {
-        RxCmdShell shell = new RxCmdShell(builder);
+        RxCmdShell shell = new RxCmdShell(builder, rxShell);
         RxCmdShell.Session session = shell.open().test().awaitCount(1).assertNoTimeout().values().get(0);
 
         when(rxShellSession.isAlive()).thenReturn(Single.just(false));
@@ -209,7 +206,7 @@ public void testAlive() throws IOException {
 
     @Test
     public void testAlive_indirect() {
-        RxCmdShell shell = new RxCmdShell(builder);
+        RxCmdShell shell = new RxCmdShell(builder, rxShell);
         shell.isAlive().test().awaitDone(1, TimeUnit.SECONDS).assertNoTimeout().assertValue(false);
 
         RxCmdShell.Session session = shell.open().test().awaitCount(1).assertNoTimeout().values().get(0);
@@ -227,7 +224,7 @@ public void testAlive_indirect() {
 
     @Test
     public void testReinit() {
-        RxCmdShell shell = new RxCmdShell(builder);
+        RxCmdShell shell = new RxCmdShell(builder, rxShell);
         RxCmdShell.Session session1a = shell.open().test().awaitCount(1).assertNoTimeout().values().get(0);
         RxCmdShell.Session session1b = shell.open().test().awaitCount(1).assertNoTimeout().values().get(0);
         assertThat(session1a, is(session1b));
@@ -238,7 +235,7 @@ public void testReinit() {
 
     @Test
     public void testReinit_exception() {
-        RxCmdShell shell = new RxCmdShell(builder);
+        RxCmdShell shell = new RxCmdShell(builder, rxShell);
         doReturn(Single.error(new IOException())).when(rxShell).open();
         shell.open().test().awaitDone(1, TimeUnit.SECONDS).assertNoTimeout().assertError(IOException.class);
 
@@ -252,16 +249,15 @@ public void testEnvironmentSetting() throws IOException {
         envMap.put("key", "value");
         when(builder.getEnvironment()).thenReturn(envMap);
 
-        RxCmdShell shell = new RxCmdShell(builder);
+        RxCmdShell shell = new RxCmdShell(builder, rxShell);
         shell.open().test().awaitCount(1).assertNoTimeout();
         verify(rxShellSession).writeLine("key=value", true);
     }
 
-
     @Test
     public void testWaitFor() {
-        RxCmdShell rxShell = new RxCmdShell(builder);
-        RxCmdShell.Session session = rxShell.open().test().awaitDone(1, TimeUnit.SECONDS).assertNoTimeout().values().get(0);
+        RxCmdShell shell = new RxCmdShell(builder, rxShell);
+        RxCmdShell.Session session = shell.open().test().awaitDone(1, TimeUnit.SECONDS).assertNoTimeout().values().get(0);
         session.waitFor().test().awaitDone(1, TimeUnit.SECONDS).assertTimeout();
 
         waitForEmitter.onSuccess(55);
@@ -273,7 +269,7 @@ public void testClose_waitForCommands() {
         BehaviorSubject<Boolean> idler = BehaviorSubject.createDefault(false);
         when(cmdProcessor.isIdle()).thenReturn(idler);
 
-        RxCmdShell shell = new RxCmdShell(builder);
+        RxCmdShell shell = new RxCmdShell(builder, rxShell);
         shell.open().test().awaitDone(1, TimeUnit.SECONDS).assertNoTimeout().values().get(0);
         shell.isAlive().test().awaitDone(1, TimeUnit.SECONDS).assertNoTimeout().assertValue(true);