joel-costigliola / assertj

AssertJ documentation
60 stars 30 forks source link

Proposed fix for #26: document how to use custom soft assertions #28

Closed paul-ko closed 9 years ago

paul-ko commented 9 years ago

Here's my initial take on documenting custom soft assertions. I'm going to hold off on tackling #27 until this one is ready to be merged in.

Also, I got the build warning below on each commit I made. Hopefully this isn't something I've introduced - let me know if I need to do anything to address it.

The page build completed successfully, but returned the following warning:

CNAME already taken: assertj.org

For information on troubleshooting Jekyll see:

https://help.github.com/articles/using-jekyll-with-pages#troubleshooting

If you have any questions you can contact us by replying to this email.

paul-ko commented 9 years ago

I just realized that my documentation doesn't address the issue I logged as #447 in assertj-core - the problem where if I call custom assertion method A softly, and A calls assertion method B, and B fails, A will continue to execute.

I see three obvious options:

  1. Ignore the problem.
  2. Sit on this pull request until a fix for the issue is released
  3. Update the pull request to include documentation of the issue and a workaround. (And then remove this information when a fix is released.)

My vote is for option 2 if you expect to release a fix relatively soon, and option 3 otherwise. Which do you prefer?

Incidentally, I came up with a workaround for the issue today that I prefer to the two I previously tried. For example, Instead of calling isNotNull in custom assertion methods and then explicitly checking that actual != null, I now call failIfNull, which is defined as below:

public void failIfNull() {
  if (actual == null) {
    failWithMessage("Object was unexpectedly null.");
  }
}

This approach causes the assert calling failIfNull to fail cleanly when executed softly. Based on your description of the issue's root cause, I'm guess that's because this approach avoids creating a new proxy context when checking for null.

paul-ko commented 9 years ago

Well, this is embarrassing. The workaround for assertj-core issue #447 I described above doesn't work the way I thought it did - I must have screwed up my testing somehow.

I was able to tweak the approach to properly work around the issue, but the workaround is no longer simple and clean enough for it to potentially be worth recommending in documentation.

joel-costigliola commented 9 years ago

Documentation updated, thanks Paul !