Comment on revision r2483 in power-matchmaker
power-ma...@googlecode.com
Details are at
http://code.google.com/p/power-matchmaker/source/detail?r=2483
Line-by-line comments:
File:
/branches/spobject-conversion/regress/ca/sqlpower/matchmaker/ColumnMergeRuleTest.java
(r2483)
===============================================================================
Line 67: protected ColumnMergeRules getTarget() throws SQLObjectException {
-------------------------------------------------------------------------------
Why does this throw SQLObjectException? What is the point of this method if
the variable is already package private?
Line 137: return getTarget();
-------------------------------------------------------------------------------
This target is not a child of the root node. The documentation on the
method you have implemented reads:
Returns an object of the type being tested. Will be used in reflective
tests being done for persisting objects. This must be a descendant of the
root object returned from {@link #getRootObject()}.
File:
/branches/spobject-conversion/regress/ca/sqlpower/matchmaker/MatchMakerSettingsTest.java
(r2483)
===============================================================================
Line 79: return mms;
-------------------------------------------------------------------------------
See the documentation for this method. This is repeated in other classes in
this commit.
File:
/branches/spobject-conversion/regress/ca/sqlpower/matchmaker/MatchMakerTranslateGroupTest.java
(r2483)
===============================================================================
Line 62: public NewValueMaker createNewValueMaker(SPObject root,
DataSourceCollection<SPDataSource> dsCollection) {
-------------------------------------------------------------------------------
To simplify the tests you should be able to implement this in
MatchMakerTestCase once.
File:
/branches/spobject-conversion/regress/ca/sqlpower/matchmaker/ProjectTest.java
(r2483)
===============================================================================
Line 142: assertEquals("Wrong type of event
fired",0,l.getPropertyChangedCount());
-------------------------------------------------------------------------------
This test is to ensure adding a process to a project fires an event. The
test has been updated to ensure adding a process to a project does not fire
a property change event. While the new check is ok to keep we are still
looking for the correct assertion to ensure a child event was fired.
Respond to these comments at
http://code.google.com/p/power-matchmaker/source/detail?r=2483
--
You received this message because you starred this review, or because
your project has directed all notifications to a mailing list that you
subscribe to.
You may adjust your review notification preferences at:
http://code.google.com/hosting/settings