[MPMD-412] Simplify stub setup by not swallowing exceptions #226
Conversation
elharo
changed the title
| setModel(model); | ||
| } catch (Exception e) { | ||
| } | ||
| InputStream in = new FileInputStream(getBasedir() + "/" + getPOM()); |
There was a problem hiding this comment.
stream should be closed also in tests
There was a problem hiding this comment.
No, if we do that the test fails when the pomReader closes the inputstream. Previously this failure was hidden by the empty catch block. This surprised me too.
There was a problem hiding this comment.
In my test stream is still open after pomReader.read(is), can you show code where is closed in pomReader.read(is)
There was a problem hiding this comment.
OK, maybe that's not where it is, but two tests broke for that inobvious reason when I initially set this up with try with resources. I had to drop the closing to make them pass.
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
There was a problem hiding this comment.
maybe we should not skip exception and find root cause
by the way all tests pass on CI, I also didn't have a problems locally
|
Previously we were suppressing the exception with an empty catch block. This seems to have hidden failures in existing tests. With PR we are no longer hiding such failures |
| @@ -427,7 +423,6 @@ public void testSuppressMarkerConfiguration() throws Exception { | |||
| // check if there's a link to the JXR files | |||
| str = readFile(generatedReport); | |||
|
|
|||
| assertTrue(str.contains("/xref/def/configuration/AppSample.html#L27")); | |||
There was a problem hiding this comment.
why this assertion was removed?
There was a problem hiding this comment.
Because as best I can tell the assert is incorrect and broken and has been for some time. That is, I don't think this string is actually expected to appear in the output. However, the bad catch block hid the failure.
There was a problem hiding this comment.
or maybe not, let me take a deeper look
There was a problem hiding this comment.
OK, asserts restored
|
Resolve #379 |
1 similar comment
|
Resolve #379 |
Removing Scm objects that were thrown away by the setter not setting anything, and pushing duplicate code up into the superclass.
This PR only changes files in src/test so it should have no effect on the correctness of the actual plugin.