Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding support for Android slim client #602

Merged
merged 9 commits into from Jan 10, 2019

Conversation

Projects
None yet
4 participants
@achmyr
Copy link
Contributor

commented Jan 29, 2015

This changes intended to remove dependency to java.beans.* package which is not supported by Dalvik VM and add packaging task for create fitnesse-slim.jar which can be added to Android app and execute fitnesse tests using slim.

Sample code to run fitnesse client inside android app

import fitnesse.slim.JavaSlimFactory;
import fitnesse.slim.SlimService;

SlimService.Options options = SlimService.parseCommandLine(new String[]{"-v", "-d", "8099"});
SlimService.startWithFactory(JavaSlimFactory.createJavaSlimFactory(options), options);

This will open socket on Android phone on port 8099 which can be accessed by Fitnesse server.

@amolenaar

This comment has been minimized.

Copy link
Collaborator

commented Jan 29, 2015

Hi, Nice commit. It would be awesome to have Slim running on android. I noticed a few things, though:

  1. The SecurityManager has been completely disabled. I assume it's not available on Android, but it would be nice to leave it in for plain Java tests.
  2. The java.beans dependency is completely gone. I'm not sure how this will work out for users currently depending on the beans framework (are there any?).
@achmyr

This comment has been minimized.

Copy link
Contributor Author

commented Jan 30, 2015

Hi,
My comments below:
1.regarding SecurityManager - you're right, it is not implemented on Android - setSecurityManager() throws Exception, getSecurityManager() returns null.
What would you suggest to put as a condition to distinguish Java/Android runtime?
I have an example, but personally I don't like this way to check. If you have some other suggestions, I would be happy to use it.

 final String vmName = System.getProperty("java.vm.name");
 if ("Dalvik".equalsIgnoreCase(vmName) || vmName.toUpperCase().startsWith("ART")) 

2.The java.beans package classes were used specifically in slim.converters package as internal implementation of text<-->Object conversion. Those new classes just copy of the real java.beans implementation, but some methods are not used at all. Would it be better to remove them or leave for compliance? Honestly, I would completely rewrite it just to have cleaner code.

P.S. This code is already used in a huge android project.

@amolenaar

This comment has been minimized.

Copy link
Collaborator

commented Jan 31, 2015

Hi,

  1. I was afraid the whole API was missing on Android. If it is there we can skip setSecurityManager if oldSecurityManager is null (defensive), or explicitly catch the exception on setSecurityManager() (make it a best effort operation / fire and forgive). I'd prefer the latter.
  2. The java.beans package has a means to register editors (application wide) via the PropertyEditorManager. It's intended to support any converter registered this way. Replacing it with an implementation in FitNesse doesn't make sense, since the registered PropertyEditors are not accessible anymore. However, I see that having a hard dependency on these beans-classes doesn't work for Android. Basically, not having ConverterRegistry depend on java.beans solves the issue, right?

I'd love to see how you make it work (with launching the android app and all). Do you have written a blog about it?

Do you have an example project on which we can experiment?

@amolenaar

This comment has been minimized.

Copy link
Collaborator

commented Feb 13, 2015

@Smudgy Do you have a test/demo app for Android?

@achmyr achmyr force-pushed the ACM64:master branch 2 times, most recently from e12e8c6 to 1a61347 Mar 26, 2015

@achmyr

This comment has been minimized.

Copy link
Contributor Author

commented Mar 27, 2015

Hi, Sorry for a delay - was busy writing fitnesse fixtures :)

I've created test app showing primitive fixture running as a part of android app. Also, updated merge with changes for SecurityManager and with latest master.
Project is here AndroidFitnesseExample

It is unclear to me still why Long is not supported as a type? Timestamps are great examples why we need it.

StringUtils was required to be added into slim-jar. I believe once you decide what to do with it build.xml should be refactored.

Please shout if you need more info on a project

@amolenaar

This comment has been minimized.

Copy link
Collaborator

commented Aug 8, 2015

Just an update: I plan to look into this, only it's not on the top of my back log.

@achmyr achmyr force-pushed the ACM64:master branch from 1a61347 to 790f8a9 Jan 21, 2016

@brucetrask

This comment has been minimized.

Copy link

commented Apr 23, 2016

I ran the example from smudgy and it works great. Thanks for that.
A question on a slightly different subject but related: Any know difficulties with running the Fitnesse Server on Android itself? I would like to use some FitLibrary features and so am not using SLIM. I believe to run FitLibrary I would have to run the whole fitnesse server on the Android device. Any known dependencies that Dalvik would not like or other know issues whtat would cause problems or should I just give it a go and see what pops up?

@achmyr

This comment has been minimized.

Copy link
Contributor Author

commented Apr 23, 2016

Hi,
I've started from intention of making server run on android, but gave up
after couple hours of adding dependencies. It may changed for M, give it a
go and let me know how it goes.
On 23 Apr 2016 20:02, "brucetrask" notifications@github.com wrote:

I ran the example from smudgy and it works great. Thanks for that.
Dumb question on a slightly different subject but related. Any know
difficulties with running the Fitnesse Server on Android itself. I would
like to use some FitLibrary features and so am not using SLIM. I believe to
run FitLibrary I would have to run the whole fitnesse server on the Android
device. Any known dependencies that would cause problems or should I just
give it a go and see what pops up?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#602 (comment)

@fhoeben

This comment has been minimized.

Copy link
Collaborator

commented Dec 22, 2018

I just came across this PR. Are you still using FitNesse, still happy with the approach in this PR?

@achmyr

This comment has been minimized.

Copy link
Contributor Author

commented Dec 26, 2018

@fhoeben
Yes, I'm happy with the approach. I've double checked with the latest Android API, and these changes still make sense

@achmyr achmyr closed this Dec 26, 2018

@achmyr achmyr reopened this Dec 26, 2018

build.xml Outdated
@@ -68,6 +68,23 @@
</jar>
</target>

<target name="slim-jar" depends="compile-slim" description="generate slim only jar file">

This comment has been minimized.

Copy link
@fhoeben

fhoeben Jan 2, 2019

Collaborator

Can you convert this to the build.gradle currently in use?

This comment has been minimized.

Copy link
@achmyr

achmyr Jan 3, 2019

Author Contributor

👍

@@ -0,0 +1,15 @@
// Copyright (C) 2003-2009 by Object Mentor, Inc. All rights reserved.

This comment has been minimized.

Copy link
@fhoeben

fhoeben Jan 2, 2019

Collaborator

Why this copyright message?

This comment has been minimized.

Copy link
@achmyr

achmyr Jan 3, 2019

Author Contributor

I believe this copyright was in the IntConverter.java, so it was inherited by copy here

This comment has been minimized.

Copy link
@fhoeben

fhoeben Jan 5, 2019

Collaborator

Just remove it please, we don't add such messages anymore and this is a new/different class not created by Object Mentor

@@ -0,0 +1,117 @@
/*

This comment has been minimized.

Copy link
@fhoeben

fhoeben Jan 2, 2019

Collaborator

Why this copyright message?

This comment has been minimized.

Copy link
@achmyr

achmyr Jan 3, 2019

Author Contributor

The PropertyEditorManager was copied over from original source in ASF. I'm open for suggestion what to put in here.

This comment has been minimized.

Copy link
@fhoeben

fhoeben Jan 5, 2019

Collaborator

If there are no modifications, just leave it as-is. Otherwise add a line to indicate copyright of FitNesse contributors.

@fhoeben

This comment has been minimized.

Copy link
Collaborator

commented Jan 5, 2019

I'm currently looking at updating the target JVM for FitNesse to Java 8 (it currently is 7), does this pose any problems for running on Android?

@achmyr

This comment has been minimized.

Copy link
Contributor Author

commented Jan 7, 2019

The JVM version update itself is not a problem. The only restriction for Android that it doesn't have the Streams API and once it has been introduced into the code, it may stop working at all.

@achmyr achmyr force-pushed the ACM64:master branch from 790f8a9 to 8a274c8 Jan 8, 2019

@achmyr

This comment has been minimized.

Copy link
Contributor Author

commented Jan 8, 2019

@fhoeben I've made changes you requested, also, the test-app was updated to the latest Android SDK. Please review

@fhoeben

fhoeben approved these changes Jan 8, 2019

@fhoeben

This comment has been minimized.

Copy link
Collaborator

commented Jan 8, 2019

I believe the changes do have an effect on the current Java Slim implementation: System.exit() no longer seems to be prevented.
When I run FitNesse.SuiteAcceptanceTests.SuiteSlimTests.SystemExitIsPrevented my test run is aborted. Does that test become green for you?

@@ -26,9 +26,19 @@ private SystemExitSecurityManager(SecurityManager delegate) {
public static void activateIfWanted() {
if (isPreventSystemExit()) {
SecurityManager currentSecMgr = System.getSecurityManager();
SystemExitSecurityManager systemExitSecurityManager = new SystemExitSecurityManager(
if (currentSecMgr != null) {

This comment has been minimized.

Copy link
@fhoeben

fhoeben Jan 8, 2019

Collaborator

I believe the security manager will be null in plain java also, therefore the system exit one will not be added.
What I've seen (in slf4j) is a check for android by getting system property: java.vendor.url turning that into lowercase and checking whether it contains android...

@fhoeben
Copy link
Collaborator

left a comment

Please ensure the SystemExitSecurityManager works in Java

@fhoeben

This comment has been minimized.

Copy link
Collaborator

commented Jan 9, 2019

On 2nd thought there seems to be little point in having a custom PropertyEditor interface. If one can adapt an existing PE to implement the new interface, one might as well just implement the Converter interface, and register the class directly, right?

fhoeben and others added some commits Jan 9, 2019

Only use property editors via reflection from main code, and don't in…
…clude the class accessing java.bean in slimJar
Merge pull request #1 from fhoeben/androidSlim
Fix for prevent system exit security manager, and alternate approach for PropertyEditors
@achmyr

This comment has been minimized.

Copy link
Contributor Author

commented Jan 10, 2019

All the changes are in now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.