Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot write code that deals with assumption violation without using internal API #390

Closed
rockwalrus opened this issue Feb 23, 2012 · 9 comments

Comments

@rockwalrus
Copy link

Since AssumptionViolatedException is internal API, you cannot write a rule, runner, etc. that react differently to assumption violations than other exceptions without crossing the boundary into internal API.

@dsaff
Copy link
Member

dsaff commented Oct 10, 2013

This is an old issue, but still quite valid. Unfortunately, I don't really want to create a copy of AssumptionViolatedException in a non-internal package.

I actually think the answer to this is to stop using package names to indicate intent of visibility or stability: create @internal and @experimental annotations, which we can remove easily in order to "promote" these hidden APIs to full public. The question, then, would be javadoc generation, and how to document that "internal" doesn't mean the same thing it once did.

@kcooney
Copy link
Member

kcooney commented Jan 24, 2014

For MultipleFailureException we created a subclass of the internal exception. See b3789b3 for details.

I could be convinced to create @Internal and @Experimental if we had some code that users could use to do static validation to ensure that they aren't being referenced (perhaps using Findbugs or https://code.google.com/p/error-prone/)

@kcooney
Copy link
Member

kcooney commented Jan 24, 2014

To clarify: I think the low-hanging fruit here is to create a subclass of AssumptionViolatedException (named AssumptionViolatedException) in an external package and deprecate the internal AssumptionViolatedException

@dsaff
Copy link
Member

dsaff commented Jan 29, 2014

@kcooney,

The external package subclass is a good idea. We should be sure, however, to also expose external API that allows an extender to know whether an exception is an assumption failure, since instanceof external.AssumptionViolatedException won't catch assumption failures from old JUnit versions.

Perhaps something like AssumptionViolatedException.isAssumptionViolation(Throwable t)?

@kcooney
Copy link
Member

kcooney commented Jan 29, 2014

@dsaff If you are talking about code outside of JUnit itself that is catching internal.AssumptionViolatedException that would be taken care of by having external.AssumptionViolatedException extend internal.AssumptionViolatedException and updating the public Assume method to throw external.AssumptionViolatedException. Newer code would of course catch external.AssumptionViolatedException

Or are you talking about code outside of JUnit itself that is throwing internal.AssumptionViolatedException? If so, I'm not sure there's much we can do; they should have used the external APIs to throw the exception.

@dsaff
Copy link
Member

dsaff commented Jan 29, 2014

@kcooney, I'm thinking about someone writing a new Rule that wants to be a good citizen and handle assumption failures. She'd be tempted to do this:

try {
   base.evaluate();
} catch (external.AssumptionViolatedException e) {

But if this Rule gets re-used on a project with an old core JUnit (or mixed together with a Rule that threw caution to the wind, and directly instantiates internal.AssumptionViolatedException), then the catch won't fire if external extends internal (which does, in general, seem like the right idea), so we should add some other way of determining whether an exception is an assumption violation, regardless of which version of JUnit threw it.

Make sense?

@kcooney
Copy link
Member

kcooney commented Jan 30, 2014

@dsaff if that rule is used in a project that used JUnit 4.11 or earlier, it would fail spectacularly because external.AssumptionViolatedException wouldn't be in the runtime classpath. And if we added a method that determined whether an exception was an assumption violation, that method wouldn't exist until 4.12, so that would also fail in 4.11.

I can't imagine code throwing internal.AssumptionViolatedException when there's a perfectly good public API for throwing it.

@dsaff
Copy link
Member

dsaff commented Feb 1, 2014

Good point about JUnit lib. Not sure what late-night state of mind produced my confusion.

@kcooney
Copy link
Member

kcooney commented Jul 13, 2014

Fixed in #818

@kcooney kcooney closed this as completed Jul 13, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants