jMonkeyEngine / jmonkeyengine

A complete 3-D game development suite written in Java.
http://jmonkeyengine.org
BSD 3-Clause "New" or "Revised" License
3.74k stars 1.12k forks source link

identifying apps in jme3-examples #1649

Open stephengold opened 2 years ago

stephengold commented 2 years ago

In PR #1591, @MeFisto94 made a suggestion for improving the test chooser: "we should probably scratch all that logic and use some annotation like @JmeExample on classes and then org.reflections to find those classes"

tlf30 commented 2 years ago

Just something to take not, org.reflections does not yet support jdk11+. https://github.com/ronmamo/reflections/issues/313

Edit: last time I tried to use it in a jdk 11 project, several errors were thrown. It looks like some of those may be fixed for jdk less than 17, but I'm not sure without further testing. It would definitely be good to do some tests to see how broken it is.

Scrappers-glitch commented 2 years ago

I have been doing something similar on one of my repositories for automating tests, but it was more like an abstract class or an interface UnitTest and it has method (invoke) and then via reflection, I chose a package to search for all classes extending this interface and debug them or debug specific classes, I can take this issue and work on it, but could you please provide more info on what is need to be done, so I know my directions, thanks.

stephengold commented 2 years ago

@Scrappers-glitch you'll need to get in touch with @MeFisto94 .

MeFisto94 commented 2 years ago

Okay, so this is a very free task as in you need to define the next steps and maybe coordinate them with us. In general the reasoning was, that the previous method was manually searching for eligble tests, so one step is certainly looking back at that and discover it's intentions.

Regarding your suggestion on interface versus annotation, that depends: In our case, almost all tests extend Application (or Simple/Legacy Application), so we could really go after that and start() and stop() them already. For this, you need to see if this works out or if there are any tests (or even multiple) that use their own main method, in which case it's still possible via reflection, but we'd need to call and discover the main method (which can be done via signature and name, or an annotation).

Then we need to find if there are implements Application (or in practice rather extends SimpleApplication), that are NO examples. And if they are, we really need an explicit annotation.

With that feedback we can then discuss what to do next, I guess :)

Scrappers-glitch commented 2 years ago

I don't prefer annotations so much, i started using them in my code recently, but I don't see any benefit rather than pumping the code with an additional dust, so I suggest a simpler and cleaner approach which is creating a new interface JmeUnitTest and inside it we create 2 methods : 1) Setup window -- for setting up the window before start. 2) Run -- for running the SimpleApplication.

And we call those in a synchronous matter (setup before run) using reflection, all classes implementing JmeUnitTest can be debugged.

and this approach will work for android too...because you can do this for example, then use an executer (another util class we will build) to execute this :

public class MyTest extends SimpleApplication implements JmeUnitTest {
@Override
public void setupWindow() {
      final JmeSurfaceView surfaceView = (JmeSurfaceView) getUserObject();
      ....
}
@Override
public void run() {
     final JmeSurfaceView surfaceView = (JmeSurfaceView) getUserObject();
     surfaceView.startRenderer(getDelay());
}
....
}

Where userObject is just a reference defined in interface JmeUnitTest for dependency injection, in this case the JmeSurfaceView object will be passed from the activity using reflection, so if you have a package test in your android app, you can simply create an activity TestActivity and another subpackage jmetest then implement your logic for calling setupWindow() and run() and depending on your logic, you will get your result, some people may prefer to implement a JmeUnitTest on a BaseAppState for example and pass some data from the main activity like the Application instance to attach that state..and here we go, you could customize and add flavors to that...

What do you think ?

Scrappers-glitch commented 2 years ago

It will be hard to debug things on android if you did the same thing but using the Application by directly calling start() or stop(), because android call these inside a View wrapper which implements a SystemListener, for example JmeSurfaceView and AndroidHarness,

EDIT: but I guess I will try and let you know, before taking other actions.

Scrappers-glitch commented 2 years ago

For the Executor is a util that will execute our JmeUnitTest classes, classes are executed in order in their package one by one waiting for the previous and checking if an internal boolean (for example isActive()) is false then the next test starts and so one till we reach the end of the files.

EDIT : We may also add an enum to filter testcases, for example ALL_PHYSICS, ALL_SHADERS, ALL_FILTERS,..etc.

stephengold commented 2 years ago

creating a new interface JmeUnitTest

Lets not call it that unless the classes which implement the interface are actually intended for unit testing. Most of the apps in jme3-examples are not unit tests. Most of them are "tech demos" or "examples", intended show off key features of the Engine and how they are intended to be used. While such apps are useful for high-level testing of functionality, they are not unit tests.

"Unit tests" (as I understand them) are tests limited to a single subsystem, such as a developer might run on new code prior to its integration into a larger codebase.

Scrappers-glitch commented 2 years ago

Alright, we can choose a good name if that's the case, may be just JmeTestcase instead of UnitTest.

Scrappers-glitch commented 2 years ago

EDIT : We may also add an enum to filter testcases, for example ALL_PHYSICS, ALL_SHADERS, ALL_FILTERS,..etc.

In this case, each enum value corresponds to a marker annotation, so for example @TestPhysics @TestFilter and so on, you can mark your generic tests through the TestExecutor util and execute them only, I will show you some examples for this later.

pspeed42 commented 2 years ago

Too often we get caught up in how we want to use something and then give it that name rather than thinking about what it actually is and giving it that name.

What do we want? We want applications that can be launched from code and will still set themselves up correctly (set app settings, etc.).

What we will use it for? JME tests, examples, demos, etc.. But that's not how we name it. It isn't just a test or just an example or just a demo. That's just how we want to use the "thing launchable from code".

So I would stick with something like Embeddable or Launchable interface with a launch() or launchDesktop() or whatever method on it that we can use from desktop apps. This would take the place of the normal static main method... and those static main methods would be replaced with something like:

public static void main( String... args ) {
    new ThisApp().launchDesktop();
}

re: @TestPhysics @TestFilter... So potentially 5,000,000 different annotations as we think of 5,000,000 different things we might want to filter one? Or the smarter way of @Test("physics"), @Test("filter). Not necessarily a fan of that either but at least is' more sane and could allow multiple values.

re: "We may also add an enum to filter testcases, for example ALL_PHYSICS, ALL_SHADERS, ALL_FILTERS" ...in case it needs to be said, those are not enum names but constant names by all normal Java naming conventions. So if you do make an enum, please use proper camel-case: AllPhysics, AllShaders... though defining filters this way is a little limiting. Better to just use a string probably.

Scrappers-glitch commented 2 years ago

I like the idea of Launchables, I need to think more of good names :-) -- EDITED.

I meant enum constants for using upper case names inside an enumeration Filter, sorry, my fault.

Or the smarter way of @test("physics"), @test("filter).

Yes, this is cleaner, better.

Scrappers-glitch commented 2 years ago

though defining filters this way is a little limiting. Better to just use a string probably.

Ofc, I will go with the strings approach, if we agreed on, thanks.

pspeed42 commented 2 years ago

re: "I meant enum constants for using upper case names inside an enumeration Filter, sorry, my fault."

Well, to be clear what I'm saying, this:

enum { MY_VALUE1, MY_VALUE2, MY_VALUE3 }

is not good Java code. This is:

enum { MyValu1, MyValue2, MyValue3 }

When a Java developer reads "MY_VALUE1" their brain should instantly think "there is a 'public static final MY_VALUE1' somewhere".

tlf30 commented 2 years ago

@pspeed42 I have to disagree on this, enum members are constant values, hence their names should be uppercase.

stephengold commented 2 years ago

Based on the examples in the Google Java Style Guide I agree with @tlf30.

pspeed42 commented 2 years ago

They are not the same kind of constants as "public static final"... and Google's style guide is at odds with actual Java and a bunch of other style guidelines. But whatever.

One way you can instantly guess the difference just by reading and the other way you can't.

pspeed42 commented 2 years ago

And yes, I know you will find this inconsistently applied.

Edit: or it's a Mandella effect but I swear I did not invent this convention out of thin air.

Scrappers-glitch commented 2 years ago

For me, i have abandoned the public static final int thing and using enums only, it is self-explanatory, but anyway this is still trivial to give names to things (as long as you gave clues on the organization's code of conduct, so contributors will stay aligned to the repository's style), there are organizations that still uses the m_ member prefix on c++ and even java code (are they not aware of this, i don't think so...but may be because they are old enough that they cannot refractor), anyway this is not the right place to discuss this.

I need to know your opinions about the approach :

To sum up i have reached this, creating :

@Test("FooBarTest") public final class TestFoo extends JmeTestApp { @Override public void launch(Object userData) { super.launch(userObject); / Desktop: configure and start the application, the user data can be anything, for example a Settings config, Screen res size, AppState object to start with */ start(); /* Android: configure and startRenderer using JmeSurfaceView by using the userData object if (!(userData instanceOf JmeSurfaceView)) { throw new IllegalStateExecption("Object must be a surface view !"); } final JmeSurfaceView surfaceView = (JmeSurfaceView) userData; surfaceView.startRenderer(0); / } / main function for desktop only, for android use JmeSurfaceView#startRenderer on an Activity to test explicitly*/ public static void main(String args[]) { new TestFoo().launch(null); } }

```java
public final class TestChooserLauncher { 
       public static void main(String args[]) {
              /** test classes with 'FooBarTest' signature inside the packages 'com.jme3.test.foobar' */
              final Package fooPackage = Package.getPackage("com.jme3.test.foobar");
              final LaunchableExecutor executor = LaunchableExecutor.executeLaunchables(new Package[] { fooPackage }, "FooBarTest", null);
       }

}
Scrappers-glitch commented 2 years ago

For Synchronicity in testing, we can use this simply (also if you think this is bad, suggest better thing) :

 public static void executeLaunchables(Package[] javaPackages, @Nullable String testSignature, Object userData) {
       // fetch all classes and place them in a launchables array list
      for (Launchable launchable: launchables) {
            ....
            new Thread(() -> launchable.launch(userData)).start();
            while (launchable.isActive()) {
               doNothing();
            }
      }
}
private static Void doNothing() { 
    return null;
}

The downside of this part may be that each launchable would create a new thread (but non the less they will never run together so you will have free space) and the boolean isActive may be not thread safe unless it's volatile, but this is current things that i have reached so far.

Scrappers-glitch commented 2 years ago

This approach supports :

And can be triggered on :

Scrappers-glitch commented 5 months ago

Here, this is the API link Jme3-testable, it works fine on desktop only; on Android it doesn't; because the final byte code is not a .class file (it's a .dex; a different story though), anyway feel free to fork and build your own test application selector on top of it, the base idea is to filter examples by annotated tags...