External version of AssumptionViolatedException

[dhasday]

This is a pull-request for issue #390.

Just subclassing and deprecating the internal version and updating references. I'm wondering if there's a better package for the external version?

[kcooney]

Thanks!

To make things more backwards-compatible, code in JUnit itself that catches AssumptionViolatedException should catch org.junit.internal.AssumptionViolatedException just in case third-party code is directly throwing org.junit.internal.AssumptionViolatedException

[dhasday]

@kcooney I didn't consider it on the initial pass, but makes sense to maintain that functionality. I pushed a changeset this morning updating anywhere that catches AssumptionViolatedException and the corresponding handlers to expect the internal version.

[marcphilipp]

One minor thing: The imports are not ordered alphabetically in a couple of places leading to unnecessary changes in some files, e.g. in ExpectException. Can you fix that?

[dhasday]

@marcphilipp Serves me right for manually updating imports. I've fixed the import ordering to match coding standards for all modified files. There were a few extra fixes for some missing/extra blank lines and static imports below imports.

[kcooney]

LGTM

@dsaff Did you want to take a look?

[dsaff]

Also LGTM!

[marcphilipp]

AssumptionViolatedExceptionTest should be moved to the new package, too.

[dhasday]

@marcphilipp What's the preferred package style for tests that use experimental features?

[stefanbirkner]

The test and the class under test should have the same package. Therefore src/test/java/org/junit/tests/experimental/AssumptionViolatedExceptionTest.java should be moved to src/test/java/org/junit/AssumptionViolatedExceptionTest.java.

[pholser]

Looking forward to this one. I think, ultimately, specific assertion/assumption mechanisms (assertEquals, assumeThat, etc.) belong outside of the core of JUnit. AssertionError and AssumptionViolatedException represent implicit contracts between test and test runner, and can be satisfied in many ways (Hamcrest, etc.) Publishing AssumptionViolatedException will encourage fuller, alternative assertion/assumption mechanisms.

[dhasday]

Alright, finally got around to moving the test class. Anything left to do before this can be merged?

[kcooney]

The Stopwatch class hasn't been released yet. Could you please update the Javadoc to use the external version of AssumptionViolatedException instead?

[dhasday]

The usage in Stopwatch is overriding a method from TestWatcher so I can't update this reference without changing existing APIs

[kcooney]

Thanks!

Besides Stopwatch, do any other public APIs use AssumptionViolationException in a method signature?

[stefanbirkner]

I found no further usage of AssumptionViolationException.

[stefanbirkner]

Could you revert this change, please. This file should not be part of the issues change list, because there is no real edit.

[dhasday]

Sure thing.

[stefanbirkner]

Thanks.

[stefanbirkner]

Looks good.

Can you please squash the commits 92c8319, 22b0d30, f7ed086 and 840d78f. These are all commits but baae295 which reorders the imports. (If you're not familiar with git rebase, please let me know.)

[dhasday]

@stefanbirkner The commits should be squashed now with a few additional updates.

I noticed that I only made aesthetic changes to TestWatcherTest and StopwatchTest, so I reverted those. Also, I guess I forgot to run tests after moving AssumptionViolatedExceptionTest so I updated the import in AllTests to fix the compile error.

[stefanbirkner]

Thanks.