Improve test suite focusing on the DAG's behavior

[ggalmazor]

This PR improves the test suite by adding new tests and by providing a new DSL to define forms directly in tests.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

The only change that could affect users is a small change in TreeReference.contextualize() that could be considered fixing of a harmless formal bug. See 11262ae for more context about this.

[codecov-io]

Codecov Report

Merging #515 into master will increase coverage by 0.2%.
The diff coverage is 14.28%.

@@             Coverage Diff             @@
##             master     #515     +/-   ##
===========================================
+ Coverage     51.17%   51.38%   +0.2%     
- Complexity     3083     3109     +26     
===========================================
  Files           247      247             
  Lines         13854    13855      +1     
  Branches       2685     2685             
===========================================
+ Hits           7090     7119     +29     
+ Misses         5922     5898     -24     
+ Partials        842      838      -4

Impacted Files	Coverage Δ	Complexity Δ
src/main/java/org/javarosa/core/model/FormDef.java	68.83% <0%> (ø)	156 <0> (ø)	⬇️
...rg/javarosa/core/model/actions/SetValueAction.java	71.15% <0%> (ø)	13 <0> (ø)	⬇️
...java/org/javarosa/core/model/April2014DagImpl.java	0% <0%> (ø)	0 <0> (ø)	⬇️
...a/org/javarosa/xform/parse/FormInstanceParser.java	72.03% <0%> (ø)	112 <0> (+1)	⬆️
.../java/org/javarosa/core/model/Fast2014DagImpl.java	0% <0%> (ø)	0 <0> (ø)	⬇️
...in/java/org/javarosa/core/model/LegacyDagImpl.java	0% <0%> (ø)	0 <0> (ø)	⬇️
...in/java/org/javarosa/xpath/expr/XPathFuncExpr.java	69.96% <0%> (ø)	208 <0> (ø)	⬇️
.../java/org/javarosa/core/model/Safe2014DagImpl.java	80% <100%> (ø)	46 <0> (ø)	⬇️
...rg/javarosa/core/model/instance/TreeReference.java	71.59% <50%> (+4.29%)	101 <0> (+12)	⬆️
...a/org/javarosa/core/services/PrototypeManager.java	79.16% <0%> (-8.34%)	8% <0%> (-1%)
... and 10 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3197cb7...977dc60. Read the comment docs.

[lognaturel]

Overall great. Most of my comments are me thinking through things and don't necessarily need action. You may like some of the comment or renaming suggestions but I don't think any are critical. The only one I'd really like to see addressed is #515 (comment).

I have to say that I'm not excited about the DSL. It's a thin wrapper so I like it somewhat more than the one proposed at #454 but I'm not convinced that it's helpful. Using it still requires understanding the XForm structure and adds on another layer to study and understand.

I agree strongly that having the form definitions inline with the tests helps organize things and understand the test's context. But I don't see the benefit of the DSL over using string representations of the actual form definitions. Can you help me understand that? I'm ok with running with it because I think we have good momentum and don't want to slow things down but I'd really like to know your thinking.

[ggalmazor]

So, regarding the DSL, here's my mindset about it:

• It's good that I need to know how XForms work to use the DSL. I woudn't want a DSL that prevents me from learning the XForms syntax.

  A great example of DSLs that work this way is jOOQ for SQL

• Right now, this DSL is super dumb, but it can be improved by providing static type checking, for example, which would make using it easier for new users e.g. the model factory takes in instance and bind objects only.

• I'd argue that form parts are more reusable if you define them using the DSL instead of using just strings.

  A common scenario in our test suite is that we have different test methods using slight variations of the same form. This makes it hard to separate and understand what's common from what's different because I have to mentally parse repeatedly whole XML docs instead of just the parts of them that I need to follow in the context of the actions and assertions of the test.

  This could be improved by abstracting away what's common and leaving only the moving parts in the test method. All this should be easier to do with factories that use the DSL instead of strings.

• Overall, having a DSL makes it easier for us to implement more sophisticated tools in the future that help us to define, verify and explain JR's high-level behavior

[lognaturel]

Thanks for describing some future plans for the DSL. I'd recommend putting some of that in a comment the same way you did with Scenario so someone else coming along later can at least see some of the planned directions. I'll remain cautiously optimistic about it. 😊

[seadowg]

I might have the wrong idea of what's happening here but should isParent be deprecated (and just call through to isAncestor) here rather than just straight renamed? It looks like this causes problems for Collect (#3536) and I'm wondering if any other clients might be hit.

[ggalmazor]

Yes, @seadowg, it was a mistake to rename it and not provide an alias, sorry! #519 already tracks this and @grzesiek2010 is working on a fix.

[seadowg]

@ggalmazor ach sorry I totally missed that!

[ggalmazor]

Don't worry ;)