#53 [PATCH] Maven plugin is not using filter props. correctly

[Felipe Leme]

I just downloaded the maven-emma-plugin-0.6 and
realized it's not passing the right values to the
<ant:filter> task. For instance, instead of passing
includes="${filterIncludes}, it passes
value="maven.emma.filter.includes".

I checked on viewcvs and the problem persists on
0.7-SNAPSHOT. But I couldn't connect to sourceforge to
create a CVS diff, so I'm sending a zip with the new
file and also a normal diff:

205c205
< <ant:filter value="maven.emma.filter.value"/>
---
> <ant:filter value="${filterValue}"/>
210c210
< <ant:filter value="maven.emma.filter.includes"/>
---
> <ant:filter includes="${filterIncludes}"/>
215c215
< <ant:filter value="maven.emma.filter.excludes"/>
---
> <ant:filter excludes="${filterExcludes}"/>
220c220
< <ant:filter value="maven.emma.filter.file"/>
---
> <ant:filter file="${filterFile}"/>

-- Felipe

[Felipe Leme]

new plugin.jelly fixing the issue...

[Felipe Leme]

Logged In: YES
user_id=129356

Just as a side note, the task is documented here:

http://emma.sourceforge.net/reference/ch02s06s02.html#filtersyntax.ANT

[Chad Woolley]

Logged In: YES
user_id=447346

I'll try to get this in. I've been kind of distracted with other stuff. The main thing is
it's such a pain in the butt to do a formal release, and I haven't automated any of
it.

[Chad Woolley]

Logged In: YES
user_id=447346

This patch was applied to plugin.jelly in revision 1.7:

http://cvs.sourceforge.net/viewcvs.py/emma/plugins/maven/maven-emma-plugin/plugin.jelly?rev=1.7&view=markup

A few comments:

* I didn't use ${filterValue}, etc as the properties, as
this patch does. First, the plugin already documents the
use of "maven.emma.filter.*" as the property format, and
this is standard for maven plugins. Second, I can't see
anywhere in the docs where a standard for these property
names are specified - "emma.filter" is used as an example,
but I think using the maven standard naming conventions for
the plugin is better. If you disagree, please share your
thoughts.

* I was going to try to release this change (painful as it
is to put a new release on sourceforge and ibiblio - I need
to look into an automation tool), but first I wanted to
write a test for this bug (emma is all about testing,
right?). However, I found that the tests fail on my new dev
box (Linux FC4 + Maven 1.1-beta-1). Don't know what
combination of this or something else broke the tests for me
(they were written under winxp maven 1.0.x), but I don't
have time to figure it out now. I'm not currently using
either maven or emma on my day job :( - so I don't have much
of an itch to scratch here. I will release it formally
sometime, but I've got other stuff to do first. There are
actually eclipse projects and scripts to manage the
deployment (a little buggy) for this in
plugins/maven/upload, but I still don't have the time to do
it right now. If someone really wants it released, they are
welcome to give it a shot! For now, you can just check it
out from CVS HEAD and plugin:install it yourself.

Thanks,
Chad

[Felipe Leme]

Logged In: YES
user_id=129356

Hi Chad,

First, thanks for applying the patch so quickly

Now, some comments:

1.Use of ${filterValue} - I chose it because that variable
has been already defined in the plugin and used to do the test:

<j:set var="filterValue" value="${maven.emma.filter.value}"/>
<j:if test="${!empty(filterValue.trim())}">
<ant:filter value="${maven.emma.filter.value}"/>
</j:if>

The standard name convention is still used, as the
filterValue receives the value of
${maven.emma.filter.value}; in theory, that variable
shouldn't be necessary and the code could be changed to just:

<j:if test="${!empty(maven.emma.filter.value.trim())}">
<ant:filter value="${maven.emma.filter.value}"/>
</j:if>

Unfortunately, that wouldn't work, as Jelly/JEXP would get
confused with the dots (.). The proper way without using an
intermediary variable (filterValue) would be something like:

<j:if
test="${!trim(context.getVariable("maven.emma.filter.value"))}">
<ant:filter value="${maven.emma.filter.value}"/>
</j:if>

(I said something like because I'm not sure if that's the
proper syntax - I also prefer to use the intermediary variable).

Anyway, which variable is used is not a big deal - what
matters is that the bug has been fixed :-)

2- Tests: I didn't realize there are any tests for the
maven plugin, my bad. Anyway, I just ran it on my FC3 +
maven 1.0.2 box and they worked - if you please upload this
particular test case here I could try to help.
BTW, it's not necessary to cd to src/plugin-test and run
'maven testPlugin'; you can just run ' maven plugin: test"
from the plugin's root directory.

3: Release - wow, I read the RELEASE.txt and the process is
really a pain. Maybe a maven-sourceforge-plugin could help,
but then there is the maven upload issue as well (I haven't
been following the maven lists for months, so I'm not sure
if they somehow automatized this process).

-- Felipe

[Felipe Leme]

Logged In: YES
user_id=129356

Chad,

Shouldn't this bug be marked as fixed?

-- Felipe

[Chad Woolley]

Logged In: YES
user_id=447346

marking fixed, still need to do a release when other pending
issues are fixed.