alexruiz / fest-assert-2.x

FEST Fluent Assertions 2.x
http://fest.easytesting.org
Apache License 2.0
400 stars 69 forks source link

onProperty support #4

Closed nfrancois closed 12 years ago

nfrancois commented 12 years ago

I cannot find this feature in fest-assert 2. The PropertySupport is avaible but unused. I think it's a required feature for fest-assert 2.0 M1 to be iso functionnal with 1.4

If with the new architecture, the design does changes a lot, I can help to implement it. I think :

WDYT ?

With 1.x this feature was not avaible for arrays, now it is.

joel-costigliola commented 12 years ago

I know the onProperty feature, I wrote it ;-)

It actually is still available in Fest Assert 2.0 but user must extract properties with PropertySupport, Alex (Ruiz), the Fest creator felt it was better to separate properties extraction from assertions.

Here's a test showing the feature (breaking) change :

// CHANGED IN FEST 2.x, was assertThat(myCollection).onProperty(propertyName).contains...
  @Test
  public void collection_assertions_on_extracted_property_values_example() {

    // extract simple property values having a java standard type
    assertThat(propertyValuesOf("name", fellowshipOfTheRing)).contains("Boromir", "Gandalf", "Frodo", "Legolas");
    assertThat(propertyValuesOf("name", fellowshipOfTheRing)).doesNotContain("Sauron", "Elrond");

    // extracting property works also with user's types (here Race)
    assertThat(propertyValuesOf("race", fellowshipOfTheRing)).contains(HOBBIT, ELF).doesNotContain(ORC);

    // extract nested property on Race
    assertThat(propertyValuesOf("race.name", fellowshipOfTheRing)).contains("Hobbit", "Elf").doesNotContain("Orc");

    // in Fest 1.x, this would have been written
    // assertThat(fellowshipOfTheRing).onProperty("name").contains("Boromir", "Gandalf", "Frodo", "Legolas");
  } 

This extract is part of fest-examples project which goal is to show fest assertions usage with real code. Extract was taken from class : https://github.com/joel-costigliola/fest-examples/blob/master/src/main/java/org/fest/assertions/examples/CollectionAssertionsExamples.java

By the way, I created fest-examples because I think users like to working code but I haven't communicated on it a lot, note that, for the time being fest-examples is built against the latest fest-assert-2.x commit.

Getting back to our discussion, we know it's a breaking change, I must say I liked onProperty as it was in 1.4 but I can understand the move as the code is clearer, the only drawback is that you can't get propertyValuesOf easily with code completion anymore (the fact that you haven't found it makes my point).

For the arrays, it is a simple matter of using PropertySupport.propertyValuesOf(String propertyName, Object[] target).

As Im' looking at it, I think PropertySupport should be in org.fest.assertions.api package and not in org.fest.assertions.internal to make it is available for users.

Last thing, I was thinking of writing regular expressions to migrate onProperty (assert 1.4) usage to propertyValuesOf (assert 2.0) to help our users (and specially me since I use onProperty at work !).

Joel

Ps : Nicolas, thanks for your interest in fest assertions, it's really cool.

nfrancois commented 12 years ago

Hi Joel

I understand the Alex's choice. For a beginner, it could be weird to that the object he assert change by calling this method. It's true using direct the class PropertySupport it's not a good idea, just because importing classes from a "internal", is scary. For me, it's mean : it's internal mechanism, do not touch, it could hurt you

I saw the class Properties in org.fest.assertions.groups that using the PropertySupport.

assertThat(extractProperty("name").from(jedis)).contains("Yoda");

It's more readable that using PropertySupport, and it's a good entrypoint for the onProperty, but maybe it should be in api package just to be more visible (I don't understand why the package is name "groups")

I will take a look on your fest-example project, maybe I can complete it. (are/have should have samples)

And you welcome, I like using Fest assert at work, so I am happy to trying contribute it :) I have anothers ideas for features, like lenient equals (at work, we using something like that with Condition ) but I thought on it before creating a issue

joel-costigliola commented 12 years ago

I think you are right, Properties is the entry point that should be used - I guess I've just learnt something ;o) I agree that it should be moved to the api package (by the way we use group to either say array or collection). I will update fest-examples to show the proper way to use Properties.

I have already added are/have examples in fest-examples, see condition_example_on_multiple_elements() test

Anyway you are very welcome to hack it (I have tried using fun examples based on Star Wars and The Lord of Rings).

Feel free to create Jira if you have new assertions ideas, I will be pleased to comment them.

joel-costigliola commented 12 years ago

I was thinking that Fest should offer a single entry point for all assertions and related stuff, that would mean Assertions class to have a static method extractProperty.

If you have configured your IDE with favorite static import (at least for eclipse), the code completion will offer all assertThat methods and extractProperty. So no need to remember that extractProperty comes from Properties.

joel-costigliola commented 12 years ago

This has been fixed in 2.0M1