[jira] [Commented] (GEODE-866) Replace use of VM.invoke(Class, String) with a lambda

["ASF GitHub Bot (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/GEODE-866?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15152848#comment-15152848 ] 

ASF GitHub Bot commented on GEODE-866:
--------------------------------------

GitHub user upthewaterspout opened a pull request:

    https://github.com/apache/incubator-geode/pull/98

    GEODE-866: Converting calls of vm.invoke to lambdas

    I automatically converted many of the calls of vm.invoke(Class, String) to use a lambda instead, as discussed. Please review this PR - I think it will be easier to review the individual commits than a review board post.
    
    There were a few roadblocks along the way:
    - I changed SerializableRunnableIF to throw an exception from the run method. That made it easier to write short lambda expressions without having to put in a try catch in the lambda. 
    - I had to modify some tests to make variables effectively final, or not refer to a static inside the lambda, and a few other issues to get all of the tests to compile and pass. See my individual commits.
    
    Not all vm.invoke calls are gone, unfortunately. There are a number of places that could not be automatically converted - mostly where the code built up an object array in a weird way and then passed it to invoke. We'll have to manually fix these.
    
    


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/apache/incubator-geode feature/GEODE-866

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-geode/pull/98.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #98
    
----
commit a3389bdc233d02eca19ad5150b52a27372697b69
Author: Dan Smith <up...@apache.org>
Date:   2016-02-11T00:09:14Z

    Changing SerializableRunnableIF to throw exception from run
    
    This makes writing tests easier, it will get converted to an unchecked
    exception.
    
    Adding throws Exception to some tests that invoked run directory on
    SerializableRunnable.

commit 5350890c10c73987e343d43c8d0ace3e17fdf7ca
Author: Dan Smith <up...@apache.org>
Date:   2016-02-11T01:49:45Z

    Decrecating all string based methods from VM
    
    These methods should no longer be used. Unfortunately, there are some
    calls that could not be automatically converted, so they cannot be
    completely removed.

commit 69024aa9946d1c2da2ca7245e6ad62f8ff025cb8
Author: Dan Smith <up...@apache.org>
Date:   2016-02-11T19:16:38Z

    Automatically converting calls of vm.invoke to lambdas
    
    This is an automatic conversion of many of the calls to vm.invoke that
    take a string to instead use a lambda.
    
    This was done using the following perl search/replace expressions on all
    dunit tests.
    perl -i -0pe 's/.\s*invoke(?:Boolean|Int|Long|)(Async|)\(\s*(\w+)(?:\.class|\.getClass\(\))\s*,\s*"([^"]+)",\s*new\s+Object\[\]\s*{([^}]*)}\s*\)/.invoke$1(() -> $2.$3($4))/mg' $1
    perl -i -0pe 's/.\s*invoke(?:Boolean|Int|Long|)(Async|)\(\s*(\w+)(?:\.class|\.getClass\(\))\s*,\s*"([^"]+)"\s*\)/.invoke$1(() -> $2.$3())/mg' $1
    perl -i -0pe 's/.\s*invoke(?:Boolean|Int|Long|)(Async|)\(\s*getClass\(\)\s*,\s*"([^"]+)",\s*new\s+Object\[\]\s*{([^}]*)}\s*\)/.invoke$1(() -> $2($3))/mg' $1
    perl -i -0pe 's/.\s*invoke(?:Boolean|Int|Long|)(Async|)\(\s*getClass\(\)\s*,\s*"([^"]+)"\s*\)/.invoke$1(() -> $2())/mg' $1

commit 8faa3841d905005fa182b2a43ac23a8bcb3eeb9b
Author: Dan Smith <up...@apache.org>
Date:   2016-02-11T00:53:15Z

    Removing tests for invoke methods I removed

commit ce74e9779d7ac5cc61545d76f4e994742ec6e093
Author: Dan Smith <up...@apache.org>
Date:   2016-02-11T00:19:01Z

    Commenting out broken code that is now a compile error
    
    These are places where the code invoke was calling a method that doesn't
    exist. These tests were either not running, or in some cases never
    checking the return value of an invokeAsync call.

commit 51c03a91750dbd547fbb7db311a7bd19ed6f90aa
Author: Dan Smith <up...@apache.org>
Date:   2016-02-11T00:52:22Z

    Misc cleanup due to stricter type checking
    
    Fixing various issues being checked by the compiler:
     - Changing generics to match callers
     - Calling invoke on the correct class
     - Fixing some ambiguous casts.
     - Fixing some method visibility issues.
     - Removing extra comma after parameters

commit 493d41b37b9077fb75e6a575220db39d41ccc8c4
Author: Dan Smith <up...@apache.org>
Date:   2016-02-11T01:24:07Z

    Dealing with some variables that need to be effectively final.
    
    Now that we are using lambdas, variables passed to the lambda should be
    effectively final.

commit 82de4d23f856c04291ef7f3b51671cadb88079dd
Author: Dan Smith <up...@apache.org>
Date:   2016-02-11T19:24:44Z

    Getting rid of some invokeBoolean calls
    
    These couldn't be automatically converted. I've changed them to lambdas.

commit 0804a1396588116caed281d2ce7303ec39aa8df1
Author: Dan Smith <up...@apache.org>
Date:   2016-02-17T00:36:52Z

    Extracting common lambdas from methods to avoid "Code to large" error
    
    After converting these tests to use lambdas, it appears that the large
    number of lambdas are overflowing some limit of the compiler. By
    extracting methods for common lambdas, the overall number of lambda
    classes required is reduced for these tests.

commit 9beed82ef44129452312cdf8021beb4b05804d86
Author: Dan Smith <up...@apache.org>
Date:   2016-02-17T20:05:55Z

    Removing a test which tests nothing.

commit 923e219f2ea8e3cd6d5baed3d585489b30c86a75
Author: Dan Smith <up...@apache.org>
Date:   2016-02-17T23:26:14Z

    Removing statics from the lambda expressions
    
    A bunch of tests were failing because the location in which static
    fields are evaluated has changed. Before, a static in the parameter list
    was evaluated on the controller. Now it is evaluated on the remote VM.
    
    Converting the statics to fields or local variables so they are captured
    and serialized.

commit f593119d2cea74ef8d75809c0c2e71b85bf0822e
Author: Dan Smith <up...@apache.org>
Date:   2016-02-18T18:37:25Z

    Removing a few more statics
    
    These are a few places I missed in my last checkin.

----


> Replace use of VM.invoke(Class, String) with a lambda
> -----------------------------------------------------
>
>                 Key: GEODE-866
>                 URL: https://issues.apache.org/jira/browse/GEODE-866
>             Project: Geode
>          Issue Type: Bug
>          Components: tests
>            Reporter: Dan Smith
>            Assignee: Dan Smith
>
> GEODE-832 allows us to use lambdas in dunit tests. We have a bunch of old tests that invoke static methods through reflection. Those should be replaced with lambdas instead.
> See this thread for more details:
> http://mail-archives.apache.org/mod_mbox/incubator-geode-dev/201601.mbox/%3CCAFh%2B7k2q58JB-dfFpGKMWC_%3DTcJOxXZRjU1G%2Bt4CuDHgTBOYwg%40mail.gmail.com%3E



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)