| Issue 145: | makePublic() proxy adds non-passed non-required arguments to proxied method call | |
| 1 person starred this issue and may be notified of changes. | Back to list |
The proxy method generated by using the makePublic() function passes all
arguments as named arguments, even if they're not passed to the test,
assuming a default of empty string even when the original private method
has no default attribute for a given argument.
This alters the behavior of the method call, which causes problems if your
private method's behavior is dependent upon passed arguments and does not
assume that an argument with an empty string value is the same as a
non-existent/non-passed argument.
To demonstrate, take this example CFC with one private method:
<cfcomponent displayname="MakePublicTest">
<cffunction name="listArgNames" access="private">
<cfargument name="argOne" type="any" required="false" />
<cfargument name="argTwo" type="any" required="false" />
<cfscript>
var argNames = '';
var key = '';
for ( key IN arguments ) {
if ( structKeyExists( arguments, key ) ) {
argNames = listAppend( argNames, key );
}
}
return argNames;
</cfscript>
</cffunction>
</cfcomponent>
Then, consider this test case, which should pass, but fails:
<cffunction name="test_listArgNames" returntype="void">
<cfscript>
var testObj = createObject( 'component', 'MakePublicTest' );
var expected = '';
var actual = '';
makePublic( testObj, 'listArgNames' );
expected = 'argOne';
actual = testObj.listArgNames( argOne = 'argOneValue' );
assertEquals( expected, actual );
</cfscript>
</cffunction>
The expected output of #testObj.listArgNames( argOne = 'argOneValue' )# is
(and it is, and can be confirmed by changing the private method to public
and testing manually):
argOne
But the output of the proxied version, after invoking makePublic( testObj,
'listArgNames' ) is:
argOne,argTwo
There seem to be two problems in mxunit.framework.PublicProxyMaker:
# The constructArgumentsTags() method uses cfparam for the default
attribute of each cfargument tag, so the proxy method will always have a
default="" attribute when the real method's cfargument doesn't even specify
a default attribute. This is fixed in my patch by only including a default
attribute for cfargument tags that have that attribute in the original method.
# In the proxy method's cfreturn call (to the "real" method) passes named
arguments for all cfargument tags even if the call does not pass all (this
is in the PublicProxyMaker.makePublic() method). This is fixed in my patch
by using "argumentCollection=arguments" in the proxy method's call to the
original method (instead of all named arguments), which creates a very
simple "pass-through" of the original arguments passed to the method call.
I found/tested this bug/fix on:
MXUnit 1.0.3
ColdFusion 8.01
Linux (Ubuntu 8.04)
I have attached both a patch to fix this bug and a copy of the fixed
mxunit.framework.PublicProxyMaker CFC.
Best,
Jamie Krug
Mar 28, 2009
Project Member
#1
marc.es...@gmail.com
Owner:
marc.esher
Mar 28, 2009
@Marc, My pleasure to provide the patch. I'm certain argumentcollection was around well before cf8, probably at least as old as MX6. After a quick search I was unable to identify when it was introduced, but Brandon Purcell made a blog post about it in June of 2003, so I think you're safe :) http://www.bpurcell.org/blog/index.cfm?mode=entry&entry=870 Best, Jamie
Mar 29, 2009
Hey Jamie, would you mind writing test cases into PUblicProxyMakerTest.cfc for these changes? I'm home alone with the kids now so I can't do it, and I had already released a new version of MXUnit just yesterday, before I read this bug. If you can get the tests in, and I can confirm at work on Monday that it all works fine on MX6.1, then I can just hold off on the release announcement and overwrite the one I put out yesterday with your fixes. Thanks!
Mar 29, 2009
Marc, Yes, I should be able to get test cases written into PublicProxyMakerTest.cfc either this evening or early tomorrow. Best, Jamie
Mar 29, 2009
awesome! Many thanks Jamie. I'll hold off a few days then get this into the latest release.
Mar 29, 2009
Marc, can you confirm the location of the latest PublicProxyMakerTest.cfc in the repo? I ran the following search, but see only branches and tags, so wasn't sure which to use: http://www.google.com/codesearch?q=PublicProxyMakerTest.cfc+package%3Ahttp%3A%2F%2Fmxunit\.googlecode\.com&origq=PublicProxyMakerTest.cfc&btnG=Search+Trunk Thanks, Jamie
Mar 29, 2009
Jamie, it's in here: http://mxunit.googlecode.com/svn/mxunit/trunk/tests/framework/ thanks, marc
Mar 30, 2009
Okay, attached is a diff including the patch for PublicProxyMaker.cfc as well as diffs for relevant testing. I've also attached a zip containing the three files modified. I've added a aPrivateMethodVariedArguments() method to tests/framework/fixture/ComponentWithPrivateMethods.cfc to use with new test cases. I've added 4 test methods to tests/framework/PublicProxyMakerTest.cfc (testMakePublicAllArgsPassed, testMakePublicArgDefault, testMakePublicMissingRequiredArg, testMakePublicRequiredArgOnly). testMakePublicAllArgsPassed() and testMakePublicArgDefault() passed before and after applying my PublicProxyMaker.cfc patch. testMakePublicMissingRequiredArg() and testMakePublicRequiredArgOnly() both failed prior to my patch, exposing the issue I aimed to fix. They pass with the patch applied. Best, Jamie
Mar 30, 2009
very cool Jamie. thanks so much. I'll give this all a whirl on mx6 when I get to work tomorrow, then release it on Wednesday. very grateful!
Mar 30, 2009
Hey, you bet, Marc. You guys developed the entire unit testing framework for me, so I'm happy to contribute one issue worth :)
Jun 30, 2010
Moving some, but not all, of these to jira.mxunit.org
Status:
Deprecated
|