android instrumentation refactor

[SierraGolf]

The main target of this refactor was to split up the rather complex logics inside the CucumberInstrumentationCore in order to have a better maintainability of that piece of code and to be able to throw in some test coverage.

In order to do that I separated out the different concerns the class was previously handling into separate classes which are now being utilised/orchestrated by the CucumberInstrumentationCore. The following aspects were extracted into their respective classes:

• converting the bundle into an "arguments" object, see Arguments (previously known as InstrumentationArguments
• running the actual tests, see CucumberExecutor
• dumping coverage data, see CoverageDumper
• waiting for the debugger, see DebuggerWaiter

Aside from moving around code and adding test coverage these are the most notable changes which happened during the refactor:

• InstrumentationArguments got renamed to Arguments
• unnecessary ordering of cucumber options has been removed (was only added to make the tests pass)
• the logic inside the AndroidResourceLoader has been simplified
• the logic of printing the summary (errors and snippets) has been moved to the AndroidLogcatReporter
• the Arguments class does not hold a reference to the Bundle anymore, but extracts all data once and is a pojo afterwards
• powermock has been added as a dependency in order to be able to test static android methods
• the robolectric version has been bumped to 2.3 in order to test logic around DexFiles
• guava has been added in order to work more conveniently with collections
• the CucumberInstrumentationCore methods onCreate and onStart have been renamed to create and start, because those methods are not callbacks but should be called in those callbacks

[SierraGolf]

Additional questions which popped up while I was doing the refactor:

1. I found that although we parse the option "strict" it is currently not supported in the android context. I think it makes sense to create a bug/feature ticket for that.
2. I think the option "monochrome", although parsed from the bundle, does not really make sense in the android context.
3. Is there any specific reason why the android module is currently not part of the released artifacts? Aside from the need to have some detailed examples and documentation (which I will take care of), I think the module is mature enough to be released publicly.
4. When writing the tests I found the method Resource#getClassName rather confusing. I think this functionality is in the wrong place. From my point of view the functionality should not reside inside each Resource implementation, but rather inside the ResourceLoaderClassFinder as some kind of auxiliary method, because the method is actually very specific to the type of resource.

[brasmusson]

@SierraGolf I'm a little curious why you say the the android module is not part of the released artefacts, in Maven Central the cucumber-android module is available in v1.1.4-v1.1.8.

[SierraGolf]

@brasmusson sorry but can you provide me a link? I wonder how the android module can be released, because it is not part of the defined modules of cucumber-jvm, it actually never was.

[aslakhellesoy]

sorry but can you provide me a link?

http://search.maven.org/#search%7Cga%7C1%7Ccucumber-android

I wonder how the android module can be released

The release process is documented in CONTRIBUTING.md

it is not part of the defined modules of cucumber-jvm, it actually never was

It's defined in pom.xml and according to git blame and Maven central it has been since 1.1.4 (Aug 2013).

[SierraGolf]

my bad. I actually only checked the first result page when searching for "cucumber". sorry for that.

while I have your attention, what do you guys think about this PR?

[aslakhellesoy]

It's a big refactoring, and I haven't had time to look at it yet. Recently I have started to spend Fridays to work full time on Cucumber-JVM (and other Cucumber-related open source projects) and I'll do my best to go through it and give feedback in the next couple of Fridays.

You know more about Android than I do, so I'll mostly be looking at whether the changes make sense overall.

Cheers,
Aslak

[SierraGolf]

Ok, thanks. I am looking forward to your feedback.

[SierraGolf]

@aslakhellesoy thanks for merging, but could you maybe take some time going through the questions I posted? thanks!

[brasmusson]

@SierraGolf

1. For consistency, I think the option "strict" should be supported in the android context.
2. As the options "monochrome" only affects (at least currently) the output from the progress formatter, the pretty formatter and the summary output, I think you are right in that it does not really make sense in the android context.
3. As discussed previously the android module is part of the released artifacts.
4. I haven't looked into the issue about Resource#getClassName

[SierraGolf]

@brasmusson thanks for taking the time. I will create pull requests for 1. and 2.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.