Build listeners.

[Luke Daley]

Hi,

While working on the changes to how tests are run I came across the  
requirement for IDEs to be able to “listen” to test runs for visual  
feedback etc.

See: https://issuetracker.springsource.com/browse/STS-551

The current approach is not quite ideal.

I have been talking to Peter Gromov from Jetbrains, who has  
implemented support for this in IDEA, on potential improvements to  
this mechanism.

Here is what we came up with…

We introduce a new interface, something like…

interface GrailsBuildListener {
        void receiveEvent(String eventName, Object[] eventArgs) throws  
Exception
}

Listeners can be added to the build via…

grails «command» -Dgrails.build.listener=my.custom.Listener

The build system instantiates an instance of this class, and passes  
all events raised during the build to the instance via the receiveEvent
() method.

This by itself obviously doesn't solve the test running issue (it does  
however raise some other interesting possibilities). The second part  
of this would be introducing a series of events that test type runners
[1] are responsible for raising.

It would probably make sense to take inspiration from JUnit's  
RunListener[2] for the series of events to raise.

Thoughts?

[1] http://github.com/alkemist/grails/blob/master/grails/src/java/org/codehaus/groovy/grails/test/GrailsTestTypeRunner.groovy
[2] http://junit.org/apidocs/org/junit/runner/notification/RunListener.html
---------------------------------------------------------------------
To unsubscribe from this list, please visit:

    http://xircles.codehaus.org/manage_email

[Graeme Rocher-3]

On Sun, Nov 8, 2009 at 1:47 PM, Luke Daley <ld@...> wrote:

> Hi,
>
> While working on the changes to how tests are run I came across the
> requirement for IDEs to be able to “listen” to test runs for visual feedback
> etc.
>
> See: https://issuetracker.springsource.com/browse/STS-551
>
> The current approach is not quite ideal.
>
> I have been talking to Peter Gromov from Jetbrains, who has implemented
> support for this in IDEA, on potential improvements to this mechanism.
>
> Here is what we came up with…
>
> We introduce a new interface, something like…
>
> interface GrailsBuildListener {
>        void receiveEvent(String eventName, Object[] eventArgs) throws
> Exception
> }

Gant and Grails already supported the Ant BuildListener interface:

http://api.dpml.net/ant/1.6.4/org/apache/tools/ant/BuildListener.html

Is this not good enough? All we would need to do is add support for
registering one via the command line.

>
> Listeners can be added to the build via…
>
> grails «command» -Dgrails.build.listener=my.custom.Listener
>
> The build system instantiates an instance of this class, and passes all
> events raised during the build to the instance via the receiveEvent()
> method.
>
> This by itself obviously doesn't solve the test running issue (it does
> however raise some other interesting possibilities). The second part of this
> would be introducing a series of events that test type runners[1] are
> responsible for raising.
>
> It would probably make sense to take inspiration from JUnit's RunListener[2]
> for the series of events to raise.
>
> Thoughts?

Seems reasonable, can we just reuse the JUnit interface though or do
we need our own?

Are you planning on implementing these changes and will they be ready for RC1?

Regards

>
> [1]
> http://github.com/alkemist/grails/blob/master/grails/src/java/org/codehaus/groovy/grails/test/GrailsTestTypeRunner.groovy
> [2] http://junit.org/apidocs/org/junit/runner/notification/RunListener.html
> ---------------------------------------------------------------------
> To unsubscribe from this list, please visit:
>
>   http://xircles.codehaus.org/manage_email
>
>
>

--
Graeme Rocher
Head of Grails Development
SpringSource - A Division of VMware
http://www.springsource.com

---------------------------------------------------------------------
To unsubscribe from this list, please visit:

    http://xircles.codehaus.org/manage_email

[Luke Daley]

On 09/11/2009, at 12:25 AM, Graeme Rocher wrote:

> On Sun, Nov 8, 2009 at 1:47 PM, Luke Daley <ld@...> wrote:
>> Hi,
>>
>> While working on the changes to how tests are run I came across the
>> requirement for IDEs to be able to “listen” to test runs for visual  
>> feedback
>> etc.
>>
>> See: https://issuetracker.springsource.com/browse/STS-551
>>
>> The current approach is not quite ideal.
>>
>> I have been talking to Peter Gromov from Jetbrains, who has  
>> implemented
>> support for this in IDEA, on potential improvements to this  
>> mechanism.
>>
>> Here is what we came up with…
>>
>> We introduce a new interface, something like…
>>
>> interface GrailsBuildListener {
>>        void receiveEvent(String eventName, Object[] eventArgs) throws
>> Exception
>> }
>
> Gant and Grails already supported the Ant BuildListener interface:
>
> http://api.dpml.net/ant/1.6.4/org/apache/tools/ant/BuildListener.html
>
> Is this not good enough? All we would need to do is add support for
> registering one via the command line.

This won't suffice.

What I am talking about is listening to the events raised by…

event("CreateWarStart", [warName, stagingDir])

for example.

>> Listeners can be added to the build via…
>>
>> grails «command» -Dgrails.build.listener=my.custom.Listener
>>
>> The build system instantiates an instance of this class, and passes  
>> all
>> events raised during the build to the instance via the receiveEvent()
>> method.
>>
>> This by itself obviously doesn't solve the test running issue (it  
>> does
>> however raise some other interesting possibilities). The second  
>> part of this
>> would be introducing a series of events that test type runners[1] are
>> responsible for raising.
>>
>> It would probably make sense to take inspiration from JUnit's  
>> RunListener[2]
>> for the series of events to raise.
>>
>> Thoughts?
>
> Seems reasonable, can we just reuse the JUnit interface though or do
> we need our own?

There won't be an interface.

There will be a series of events…

e.g. event("TestStarted", ["someTest"])

What events need to be raised can be taken from that interface though.

> Are you planning on implementing these changes and will they be  
> ready for RC1?

Yes. This blocked my work on test-app, so I need to get this done so I  
can get that done.


---------------------------------------------------------------------
To unsubscribe from this list, please visit:

    http://xircles.codehaus.org/manage_email

[Luke Daley]

http://github.com/alkemist/grails/tree/BuildListening

There is just one commit…

http://github.com/alkemist/grails/commits/4f8b81bea037afbc515664017cb446ae45b423a0

That branch forks from current grails/master HEAD so can be pulled  
without getting any of my other changes.
---------------------------------------------------------------------
To unsubscribe from this list, please visit:

    http://xircles.codehaus.org/manage_email

[Peter Ledbrook]

> http://github.com/alkemist/grails/tree/BuildListening

Any new command line system properties should be supported in
BuildConfig.groovy too. There is a "getPropertyValue" closure in
_GrailsSettings.groovy to do just this. I would also suggest you
support multiple listener classes.

It's a bit unfortunate that you have to create a new interface, but
you're right, there's no way to catch events from the event() closure
with a normal Ant build listener. An improvement for the future maybe,
so that people can drop in existing listener classes from their Ant
builds.

Finally, it's neater to use the class literal in BuildConfig.groovy
(and the classpath should be fine), so I would accept String or Class
values for the cli listener property.

Cheers,

Peter

---------------------------------------------------------------------
To unsubscribe from this list, please visit:

    http://xircles.codehaus.org/manage_email

[Luke Daley]

On 09/11/2009, at 6:37 PM, Peter Ledbrook wrote:

>> http://github.com/alkemist/grails/tree/BuildListening
>
> Any new command line system properties should be supported in
> BuildConfig.groovy too. There is a "getPropertyValue" closure in
> _GrailsSettings.groovy to do just this. I would also suggest you
> support multiple listener classes.

Good point. Adding this to BuildSettings now.

> It's a bit unfortunate that you have to create a new interface, but
> you're right, there's no way to catch events from the event() closure
> with a normal Ant build listener. An improvement for the future maybe,
> so that people can drop in existing listener classes from their Ant
> builds.

I don't think this is really that desirable.

I can't see how Ant's BuildListener and BuildEvent classes could be  
used to support the custom events of the type that the Grails build  
system raises. You could subclass these classes, but then you aren't  
any better off.

Also, since GrailsBuildEventListener raise Grails custom style events  
for the events raised by Ant then you don't _need_ to plugin in an Ant  
BuildListener… you can use the Grails style build listener for the  
same effect.

> Finally, it's neater to use the class literal in BuildConfig.groovy
> (and the classpath should be fine), so I would accept String or Class
> values for the cli listener property.

Will do.


---------------------------------------------------------------------
To unsubscribe from this list, please visit:

    http://xircles.codehaus.org/manage_email

[Peter Ledbrook]

> I don't think this is really that desirable.
>
> I can't see how Ant's BuildListener and BuildEvent classes could be used to
> support the custom events of the type that the Grails build system raises.
> You could subclass these classes, but then you aren't any better off.

Fair point. I think it might still be nice to have a mechanism for
adding standard build listeners directly to Gant, but I don't have a
particular use-case in mind. Definitely low priority unless someone
really, really needs it.

Cheers,

Peter

---------------------------------------------------------------------
To unsubscribe from this list, please visit:

    http://xircles.codehaus.org/manage_email

[Luke Daley]

On 09/11/2009, at 6:37 PM, Peter Ledbrook wrote:

> Any new command line system properties should be supported in
> BuildConfig.groovy too. There is a "getPropertyValue" closure in
> _GrailsSettings.groovy to do just this. I would also suggest you
> support multiple listener classes.

http://github.com/alkemist/grails/commits/836416cc46ddc10cc6b7da0feb5b050325a4db32

> Finally, it's neater to use the class literal in BuildConfig.groovy
> (and the classpath should be fine), so I would accept String or Class
> values for the cli listener property.

Classpaths are bad here if the class is coming from the Grails project.

The BuildSettings class loader doesn't seem to have access to the compiled Grails classes.

None the less, class literals are supported.


---------------------------------------------------------------------
To unsubscribe from this list, please visit:

    http://xircles.codehaus.org/manage_email

[Luke Daley]

http://github.com/alkemist/grails/tree/BuildListening

Comments?

---------------------------------------------------------------------
To unsubscribe from this list, please visit:

    http://xircles.codehaus.org/manage_email

[Graeme Rocher-3]

Looks ok, you don't seem to have wired it into the build yet though. I
assume this comes later ;-)

Cheers

On Thu, Nov 12, 2009 at 1:16 PM, Luke Daley <ld@...> wrote:

> http://github.com/alkemist/grails/tree/BuildListening
>
> Comments?
>
> ---------------------------------------------------------------------
> To unsubscribe from this list, please visit:
>
>    http://xircles.codehaus.org/manage_email
>
>
>

--
Graeme Rocher
Head of Grails Development
SpringSource - A Division of VMware
http://www.springsource.com

---------------------------------------------------------------------
To unsubscribe from this list, please visit:

    http://xircles.codehaus.org/manage_email

[Luke Daley]

If you mean actually using this stuff for the testing, yes that will come later once this is all approved.

If you mean actually having events sent to these listeners, that is done…

http://github.com/alkemist/grails/commit/34927f38cc3e37b9fe476b7000e6a0d5d310e115#diff-4

On 12/11/2009, at 11:13 PM, Graeme Rocher wrote:

> Looks ok, you don't seem to have wired it into the build yet though. I
> assume this comes later ;-)
>
> Cheers
>
> On Thu, Nov 12, 2009 at 1:16 PM, Luke Daley <ld@...> wrote:
>> http://github.com/alkemist/grails/tree/BuildListening
>>
>> Comments?
>>
>> ---------------------------------------------------------------------
>> To unsubscribe from this list, please visit:
>>
>>    http://xircles.codehaus.org/manage_email
>>
>>
>>
>
>
>
> --
> Graeme Rocher
> Head of Grails Development
> SpringSource - A Division of VMware
> http://www.springsource.com
>
> ---------------------------------------------------------------------
> To unsubscribe from this list, please visit:
>
>    http://xircles.codehaus.org/manage_email
>
>

---------------------------------------------------------------------
To unsubscribe from this list, please visit:

    http://xircles.codehaus.org/manage_email

[Peter Ledbrook]

> http://github.com/alkemist/grails/tree/BuildListening
>
> Comments?

You can use the getPropertyValue() method for
"grails.build.listeners". It may be worth converting the config object
to properties when the config changes. Just to avoid multiple calls to
config.toProperties() whenever you want to use getPropertyValue().

On a side note, I can't believe we have a property called
"grails.default.plugin.set" that we require to be a List. That's just
gratuitous messing with people's heads :)

Otherwise, all looks good to me Mr. Daley.

Cheers,

Peter

---------------------------------------------------------------------
To unsubscribe from this list, please visit:

    http://xircles.codehaus.org/manage_email

[Luke Daley]

On 13/11/2009, at 12:11 AM, Peter Ledbrook wrote:

>> http://github.com/alkemist/grails/tree/BuildListening
>>
>> Comments?
>
> You can use the getPropertyValue() method for
> "grails.build.listeners". It may be worth converting the config object
> to properties when the config changes. Just to avoid multiple calls to
> config.toProperties() whenever you want to use getPropertyValue().

I actually can't because I accept Class values.

> On a side note, I can't believe we have a property called
> "grails.default.plugin.set" that we require to be a List. That's just
> gratuitous messing with people's heads :)

LOL

> Otherwise, all looks good to me Mr. Daley.

Great, it's done so merge at will.

I have run it past Peter Gromov.


---------------------------------------------------------------------
To unsubscribe from this list, please visit:

    http://xircles.codehaus.org/manage_email

[Luke Daley]

Should this feature be documented in the manual?

On 13/11/2009, at 12:11 AM, Peter Ledbrook wrote:

>> http://github.com/alkemist/grails/tree/BuildListening
>>
>> Comments?
>
> You can use the getPropertyValue() method for
> "grails.build.listeners". It may be worth converting the config object
> to properties when the config changes. Just to avoid multiple calls to
> config.toProperties() whenever you want to use getPropertyValue().
>
> On a side note, I can't believe we have a property called
> "grails.default.plugin.set" that we require to be a List. That's just
> gratuitous messing with people's heads :)
>
> Otherwise, all looks good to me Mr. Daley.
>
> Cheers,
>
> Peter
>
> ---------------------------------------------------------------------
> To unsubscribe from this list, please visit:
>
>    http://xircles.codehaus.org/manage_email
>
>

---------------------------------------------------------------------
To unsubscribe from this list, please visit:

    http://xircles.codehaus.org/manage_email