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

Open
#69 5 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
5/5
Estimated time
Over a week
Newbie friendliness
30/100
Issue type
Feature
Clarity
Mostly clear
Activity status
Stale
Tech stack
java
Domain
tooling

Research direction

Start by reviewing issue 67 and assertj-core issue 641, then compare the two linked navigation templates with the generator's current property and iterable templates. Determine how the NavigationListAssert support and a compatibility flag affect generated APIs, and verify that generated assertions provide the navigation methods without breaking existing methods.

Written by the indexing model from the issue text.

Description

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):

Dominant language
Java
Stars
72
Forks
47
No merged PRs in 30d

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from assertj/assertj-generator

All issues in assertj/assertj-generator

Similar issues

More Java issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.