| 7029503206e4c89fe167c3389c1062d89cf35c52 |
|
19-Nov-2015 |
Paul Duffin <paulduffin@google.com> |
Refactoring to make more testable
Prior to making a change in the behaviour to fix an issue with
the quoting of command line arguments this refactors the
existing code and adds some tests to illustrate the existing
behaviour. This will minimize the chances that the fix will
break existing code.
The changes are explained below in more detail.
This applied a couple of common refactorings to a number of
classes:
* Where classes were creating a lot of subsidiary objects in
their constructor I switched to using the dependency injection
pattern (i.e. passing in parameters to the constructor). That
makes it easier for tests to override the behaviour.
* Where classes were being given a huge object (e.g. Run) but
only required a small number of it's fields I replaced the Run
constructor parameter with the individual parts.
LocalTarget - Replaced Run with Log, Mkdir, Rm.
Run - Passed in more parameters into the constructor, moved the
code that created those objects into Vogar.run(). Also, make the
vogarJar() method work when running from a class directory to
allow it to work from within IDEA as well as when running from
jars.
SshTarget - Removed a bit of duplication of the command line
prefix used to run a script remotely.
Target - Method defaultDeviceDir() was removed and the
implementations were made static and accessed directly from
Vogar. That is necessary to break a dependency cycle; AdbTarget
required AndroidSdk, which required DeviceFileCache, which
required runnerDir, which required Target.defaultDeviceDir().
Previously, that was broken by passing the DeviceFileCache to
the AndroidSdk after construction using the setCaches() method
but that has been removed as part of this refactoring.
Vogar - Made the constructor and parseArgs(String[]) visible for
testing. Moved some of the code from Run's constructor here to
create the values to be passed into its expanded constructor.
TargetType was added to encapsulate the defaultDeviceDir() for
each Target implementation class. That allowed the cycle
described in Target to be broken.
AdbTarget - Replaced Run with individual fields. Moved
functionality from AndroidSdk that's only used by this class
into here (waitForDevice(), ensureDirectory(File), remount(),
rm(File), forwardTcp(int), push() and pull()). Bringing in
push() also required moving the pushCache from AndroidSdk as
well. Added DeviceFilesystem as a parameter to avoid having to
have a reference to AndroidSdk.
AndroidSdk - Moved logic that runs shell commands to calculate
the compilationClasspath and androidJarPath out of the
constructor and into a static method to allow tests to pass
dummy values straight into the constructor. Marked constructor
as VisibleForTesting. Removed setCaches(..) method and
pushCache, passed the HostFileCache straight into the
constructor, marking the dexCache field as final. Added explicit
exception when can't find platforms directory rather than
NullPointerException. Moved methods that are only used by either
AdbTarget or DeviceFileCache into those classes.
DeviceFileCache - Replaced AndroidSdk with DeviceFileSystem and
moved cp(File, File) and mv(File, File) from AndroidSdk into
here.
DeviceRuntime - Added a Supplier<String> parameter to provide
the device user name in order to allow tests to provide their
own. That is needed because otherwise the test will attempt
to connect to a device to determine the user name.
Command - Added method to access the args for testing, moved the
special processing of "-c" option from start() into a method
called from the constructor. That allows the test to verify the
processing of that without actually executing the command.
Cleaned up unused workingDirectory method and some minor
warnings.
Added some tests and fixed a compile issue with the
JUnitRunnerTest.
Change-Id: Ib1676ee19a4f0e7a8944b2708a6dbe3899d1d292
/external/vogar/src/vogar/android/DeviceFilesystem.java
|