Change AssumptionViolatedException to not set the cause to null; fixes issue #494

[kcooney]

No description provided.

[marcphilipp]

I think we should ask @mmichaelis who reported #494 to take a look and make sure we solve his problem, shouldn't we?

[mmichaelis]

Thanks for referring to me... and actually thanks for fixing! Yes, I think the suggested solution works as it does not overwrite "this" cause in Throwable by accident.

The only thing to add: I think a test-case is missing which would show the failure in the previous version. Suggested test:

@Test
public void causeCanBeInitializedLater() {
  Throwable cause = new Exception();
  AssumptionViolatedException e = new AssumptionViolatedException("invalid number");
  // Old behavior: Following statement would raise IllegalStateException: "Can't overwrite cause"
  // triggered by Throwable#initCause().
  e.initCause(cause);
  assertThat(e.getCause(), is(cause));
}

[kcooney]

@mmichaelis I would think your proposed test would be covered by the tests that @marcin-lawrowski wrote that checked for a null cause. Am I missing something?

[mmichaelis]

@kcooney No, that's not enough as you cannot distinguish via getCause() the states

• cause unset (Throwable will mark this state with cause=this) from
• cause set to null
  as can be seen in Throwable both states would reveal null as result:

public Throwable getCause() {
  return (cause==this ? null : cause);
}

[kcooney]

@mmichaelis got it. Updated one of the tests we added to call initCause and check that the cause was set.

[mmichaelis]

@kcooney Great. Thanks a lot!

[marcphilipp]

@kcooney We should mention this fix in the release notes. I have already moved them from the wiki into the repository. Do you have time to do it?

[kcooney]

@marcphilipp Filed #987 to update the release notes.

[marcphilipp]

Great, thanks!