I'm mailing this to [email protected] and cc'ing to [email protected], because it's relevant to all, I think.
On 17 July 2013 08:28, GESCONSULTOR - Óscar Bou <[email protected]>wrote: > > Just a question related to yesterday's BDD implementation (really useful > links on the documentation you wrote). If the relationships are going to be > managed by DN, there could be a lot of @unit tests (all those where bidir > relationships are used) that could fail, couldn't it ? > > Yup, that's true I think. I've just been through Estatio to remove these helpers. There were one or two unit tests that broke... that's probably more an indication that we don't have nearly enough coverage yet. What I did in the unit tests - in the given portion - is explicitly set both sides of the relationship; see for example [1]. In one case [2] I had to remove an assert because the other side is not maintained automatically (more on this below) I deleted one test [3] that is now tested through the BDD spec. > > Just to clarify it before refactoring, an example with the Agreement - > Roles relationship would be refactored this way: > > Agreement.java: > > The relationship, field, getter and setter remains identical (with the > same DN annotation): > [snip] > > Remove the following bidir contract methods: > > public void addToRoles(final AgreementRole agreementRole) { > } > > public void removeFromRoles(final AgreementRole agreementRole) { > } > > correct. > > AgreementRole.java: > > No changes on field, getter, setter and annotations (despite the > @javax.jdo.annotations.Column(name = "AGREEMENT_ID") is optional, just > for changing the name of the generated table column, isn't it?): > correct. > [snip] > > > Remove the bidir contract methods: > > public void modifyAgreement(final Agreement<?> agreement) { > } > > public void clearAgreement() { > } > > correct. > > > > So, basically, with just the field, getter, setter and required DN > annotations on each side would be enough for production system and > integration tests (if the collection is a Set or SortedSet), wouldn't it? > correct. > > As I told at the beginning, also for unit tests? Agreed, some unit tests may break (see my notes above). Given that relationships are always modified through actions, I am considering adding in additional code, clearly marked as being just for unit tests... something like: setAgreement(agreement); // for unit tests only; JDO will automatically maintain this if(!agreement.getRoles().contains(this)) { agreement.getRoles().add(this); } It's ugly, I know, but I think I'd rather one line of commented ugliness than all the noise of those helper methods. > It would avoid to load all DN stuff. I've seen your InMemoryDB class and > the blog post commenting about it, but still not sure about if it's going > to be enough for testing Domain behavior / actions. > That class is just a utility, something I knocked up as a way to set up mock service/repo expectations. I don't think it has any impact on this discussion > > As you are currently making the tests on Estatio, which kind of tests must > be done as @integration with current Isis implementation, and for which > ones the faster @unit scope can be used? > for now, I'm just focusing on writing @integration specs. I know that puts me in conflict with JBRainsberger [4], but it mitigates the risk of misunderstanding what JDO/DN is doing under the hood. What I have found is that, with an @integration spec passing, then when I change the tag to @unit, JMock gives me useful information on the expectations that are missing. I can then copy these into new sections of the step defs. I've done this in Isis' example app [5], but haven't yet done so Estatio. > > > We are going to start to make some tests regarding relationships > (including 1-1 relationships, with needed flush() in our previous tests). > I'll keep you informed. > > I didn't take out the modify/clear helpers for 1:1 bidir relationships, on the basis that (a) I didn't have a test to check for regression and (b) if DN does maintained the opposing rel (as it probably does), then it wouldn't matter if Isis also set that rel with the same value also. Let me know what you find out. Thx Dan > > Thanks, > > Oscar > > > [1] https://github.com/estatio/estatio/commit/2dfd0903718a3ad769a02e4908ef45edaefae0f9#diff-21 [2] https://github.com/estatio/estatio/commit/2dfd0903718a3ad769a02e4908ef45edaefae0f9#diff-23 [3] https://github.com/estatio/estatio/commit/2dfd0903718a3ad769a02e4908ef45edaefae0f9#diff-18 [4] http://blog.thecodewhisperer.com/2010/10/16/integrated-tests-are-a-scam [5] https://github.com/apache/isis/blob/94ccda3a3f43665e42abb34368a513ff9dccb06f/example/application/quickstart_wicket_restful_jdo/integtests/src/test/java/integration/specs/todoitem/ToDoItemStepDefs.java#L94
