switch the property/field templates over to use the 'navigation' approach by default?

[jstrachan]

Once this PR is merged and issue fixed https://github.com/joel-costigliola/assertj-assertions-generator/issues/67 along with the NavigationListAssert class in assertj-core: https://github.com/joel-costigliola/assertj-core/issues/641 it would be easy to switch the OOTB templates in the generator to use the 'navigation' model approach.

So that for every property of an object we generate exactly one navigation method that lets users chain assertions together; reusing all the methods from ListAssert and NavigationListAssert for iterable properties or the custom typed assertions for other properties.

This would result in much smaller code generated for the assertion classes and much more power. The only downside is some of the current methods would be a tad more verbose.

e.g. for a generated assertion class with a name property the current generated code would be:

// current generated code
assertThat(person).hasName("James");

// new minimal code
assertThat(person).name().isEqualTo("James");
assertThat(person).name().contains("m");
...

Ditto for iterable properties right now we generate a few methods which are mostly already included in ListAssert. e.g. all these assertion methods are available on any generated iterable property method: https://github.com/jstrachan/assertj-core/blob/7b1d079edeb5ac984cebbed77a65025718ee7673/src/test/java/org/assertj/core/navigation/ListNavigation_Test.java#L56-L62

Removing the old generated methods would break folks code I guess; so maybe we should add a flag to switch to the new more concise model?

If you are interested; here are the 2 new templates (which need minor tweaks to reuse the https://github.com/joel-costigliola/assertj-core/issues/641 code):

• for general properties/fields: https://github.com/jstrachan/fabric8/blob/5f99b82696856e9c5614ff5a728adfbeaab96751/components/kubernetes-assertions/src/main/assertj-templates/navigation_template.txt
• for iterable properties/fields: https://github.com/jstrachan/fabric8/blob/6e333451afceec3308e6869f39a14c3e569d837b/components/kubernetes-assertions/src/main/assertj-templates/has_elements_assertion_template_for_iterable.txt

[jstrachan]

am happy to submit a PR for the above if you like

[joel-costigliola]

#67 and https://github.com/joel-costigliola/assertj-core/issues/641 have been fixed, it should be now possible to fix this issue.

I'm not sure I want to switch to navigation assertions by default because it makes simple usage less direct, in your example one now needs two methods completion to get to the assertion. I believe these simple usages are a majority.

What we could do is add some configuration to generate navigation assertions (either as additional assertions or being the default).

thoughts ?

[jstrachan]

having a configuration option to enable the navigation methods would be really nice - it'd be easier for folks that them maintaining their own templates

[joel-costigliola]

A progress note : this much more work than I expected, the main thing is to resolve the correct assert type got any given type. This implies some (well justified) refactoring.

One cannot just find the assert type by adding Assert and changing the package to org.assertj.core.api, think of Set, that would give org.assertj.core.api.SetAssert whereas it should have been org.assertj.core.api.IterableAssert.

No big deal, just lot of work.

[cal101]

Moin!

I am using assertj since fest times and it's a great piece of work! I want to do navigation style asserting on some complex compiler abstract syntax tree. Until i make up my mind of how to structure it i am writing the initial assert classes by hand. I use the appoach of extending the standard asserts for easy access. But what i found is that i want and need the standard methods but my custom navigation methods get lost in the huge list of inherited methods. Can we put them into different "namespaces" and enhance assertj to switch between them for composition classes?

assertThat(person).__switchToStandard().isInstanceOf(X.class).__switchToDomain().name().isEqualTo(...)

Opinions?

Cal

P.S. And yes it must be the names above. ;-)