Add Result#getAssumptionFailureCount #1294
Conversation
Fix unit test assumeWithExpectedException: it was throwing AssumptionViolatedException thus being skipped (see junit-team#98).
| assumeTrue(false); | ||
| public static class TestClassWithAssumptionFailure { | ||
|
|
||
| @Test(expected = IllegalArgumentException.class) |
There was a problem hiding this comment.
I know it was there before, but the expected parameter is confusing and should be removed.
There was a problem hiding this comment.
It was part of the scenario. It comes from the example given in the bug report: #98
|
This changes the serialized form of this class. Do we know if IDEs or build tools serialize it? |
|
Good catch, @kcooney! I guess it's okay to add an instance variable, though. On the JVM with the old version of the class this field should just get ignored. But we should write a test for that scenario. |
|
@alb-i986 Can you add such a test? Please let me know if you need help. |
|
Here is how we worked around this problem last time: f6149a9 Due to that fix we might be safe. Edit: just in case, |
| @@ -156,13 +166,15 @@ public RunListener createListener() { | |||
| private static final long serialVersionUID = 1L; | |||
| private final AtomicInteger fCount; | |||
| private final AtomicInteger fIgnoreCount; | |||
| private final AtomicInteger assumptionFailureCount; | |||
There was a problem hiding this comment.
I'm reasonably sure that if you add this, you'll be breaking serialization. It's possible that new code will be able to read objects serialized with old code (the tests in ResultTests.java check for that) but code linked against 4.12 won't be able to read Result objects serialized by versions after 4.12. That may break some build tools or IDEs.
Could we instead not add this field, and have the Result(SerializedForm) constructor set a field to indicate that the assumption failure count cannot be determined?
Edit: Actually, since you didn't modify SerializedForm.serialize(), this change won't break serialization, but the number of assumption failures will be lost when you serialize and deserialize. So with your current code, a deserialized Result instance will throw a NullPointerException when you call getAssumptionFailureCount()
|
According to http://www.jguru.com/faq/view.jsp?EID=5064 we can safely add new fields while still allowing code running with older versions of the class to read serialized data written by the newer version. So we just need to make sure that the class behaves well when it is deserialized from a binary form that was written by an older version (which didn't know about this new field). I suggest when this happens, calling |
| @@ -87,6 +90,13 @@ public int getIgnoreCount() { | |||
| } | |||
|
|
|||
| /** | |||
| * @return the number of tests skipped because of an assumption failure | |||
There was a problem hiding this comment.
Could you please add:
@since 4.13
|
@alb-i986 did you have any time to fix the issues no this pull soon? If not, it won't make 4.13 |
|
@kcooney when is 4.13 due? |
|
@alb-i986 we don't have a due date, but it has been over two years since 4.12 was released, and there are few remaining open issues targeted to 4.13 |
|
bump, 4.13 is almost there... |
|
@alb-i986 Any news? Please let us know if you can do the requested changes. |
|
Replaced by #1530 |
My original intent was to fix unit test assumeWithExpectedException: it was throwing AssumptionViolatedException thus being skipped (see #98).
It led me to adding Result#getAssumptionFailureCount.