Test Failures Hidden by double-expanded-properties Bug

[Stefan Bodewig <bo...@apache.org>]

Hi all,

macrodef'ed tasks used to run their attributes through property
expansion twice (or even more often if the nested tasks of a macro have
been macros themselves).

https://issues.apache.org/bugzilla/show_bug.cgi?id=42046

The reason is they get expanded once for the macro instance and once
again as attributes of the nested tasks after macro attributes have been
expanded.

This is fixed now.

My initial problem with the patch was that it made two of Ant's AntUnit
tests fail, it turns out that in both cases the tasks tested don't work
the way the tests wants them to and the issues were hidden by
double-expansion of properties.

Case 1: tasks/property-test.xml testNestedExpansionHonorsImmutability
=====================================================================

I've already committed a commented-out alternative assertion using
logging and assertLogContains that shows the value of bar.y is not y
(but ${bar.x} which expanded to y on the second expansion).  Here the
issue hid a bug - assuming the tested behavior is one we want to work.

Case 2: tasks/propertyhelper-test.xml testLoadProperties
========================================================

Now XtestLoadPropertiesWithObjects

This was more subtle.

${object} expands to an Object rather than a String.  object2 OTOH will
be the string "java.lang.Object(...)" that is ${object}.toString().
This happens inside ResolvePropertyMap.java line 140 and just is the way
nested expansions of loadproperties works.

Prior to the fix ${object} in 

    <au:assertPropertyEquals name="object2" value="${object}" />

was expanded for the MacroInstance representing the assertPropertyEquals
instance.  MacroInstance supports attributes only via DynamicAttribute
which also means it only supports string-values attributes and so the
value attribute was set to "java.lang.Object(...)".

Now the macro expands to

      <au:fail message="...">
        <equals arg1="${object2}" arg2="@{object}"
                casesensitive="true"/>
      </au:fail>

before any property is expanded and since <equals> supports attributes
of type Object arg1 is "java.lang.Object(...)" while arg2 is a real
object and equals evaluates to false.

>From this I take away:

* <assertEquals> and <assertPropertyEquals> should support the
  forceString attribute of <equals>.  With that set to true the original
  assertion holds true.

* <loadproperties> does not work as the test expected for properties
  with non-string values but this is very much by design.  I don't think
  the test was correct and we should remove it (or replace it with a
  forcestring="true" version).

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org