Non-determinism when choosing overload if overloads involve varargs

[nathanwonnacott]

There are some issues with the way AnnotationKeywordExtractor handles overloaded methods when an overload contains a vararg.

The specific example I ran into is from the testfxlibrary.

They have a new keyword defined as:

@RobotKeyword
@ArgumentNames({"className", "distinctiveName=null", "args=null"})
public void startApplication(String className, String distinctiveName, String ... args) {
    //blah blah blah
}

@RobotKeywordOverload
public void startApplication(String className) {
    //blah blah blah
}

From my understanding of the AnnotationKeywordExtractor code, it will load all of the methods from each keyword class and if it has a @RobotKeyword or @RobotKeywordOverload method, it will create a DocumentedKeyword from them and add them to a map (with the method name as the key). If the name already exists in the map, then it creates an implementation of DocumentedKeyword that is a wrapper around the existing DocumentedKeyword in the map, but first checks the number of arguments, and if it matches its number of arguments, it calls its invoker, otherwise it calls the wrapped invoker.

So, assuming that the version of startApplication that takes 1 argument was loaded last, you essentially end up with this pseudo-code in your DocumentedKeyword.execute(Object[] arguments) method:

if num arguments == 1:
    call startApplication(String)
else
    call startApplication(String, String, String...)

So if I call the keyword with 4 strings, then it will call the correct startApplication method and the argument grouping code will combine the last two strings into an array of strings.

However, the keyword methods are loaded by a call to keywordBean.getClass().getMethods() (AnnotationKeywordExtractor:32). The documentation from Class.getMethods states:

The elements in the returned array are not sorted and are not in any particular order

If the methods are loaded in a different order, you can end up with the following pseudo code:

if num arguments == 3:
    call startApplication(String, String, String...)
else
    call startApplication(String)

Now if you call the keyword with 4 strings, then it will try to run startApplication(String). The argument grouping code will group the 4 strings into an array of strings, attempt to invoke the method that only takes one string and throw an IllegalArgumentException.

The result for me is that my tests work every time on my windows desktop, but they only work about half of the time on my team's Linux Jenkins server.

[nathanwonnacott]

There may be a more elegant way to handle this, but there is a simple one line fix that I think will work.

It seems to me that at least in the case where all parameters are strings (and I believe that currently there is only varargs support for methods that contain only strings see issue #12) then as long as the methods with the fewest number of arguments are loaded first, then it will work.

So if you just change AnnotationKeywordExtractor.extractKeywords(Object) to:

public Map<String, DocumentedKeyword> extractKeywords(final Object keywordBean) {
  Map<String, DocumentedKeyword> extractedKeywords = new HashMap<String, DocumentedKeyword>();
  Method[] methods = keywordBean.getClass().getMethods();
  //Add methods with the most parameters first for funky varargs reasons
  //Also, probably come up with a better comment than this
  Arrays.sort(methods, (m1, m2) -> -Integer.compare(m1.getParameterCount(), m2.getParameterCount()));
  for (final Method method : methods) {
     if (method.isAnnotationPresent(RobotKeyword.class) || method.isAnnotationPresent(RobotKeywordOverload.class)) {
      createOrAddKeyword(extractedKeywords, keywordBean, method);
    }
  }
  return extractedKeywords;
}