search

eclipse / lsp4mp

Technology lsp4mp
Eclipse Public License 2.0
22 stars 27 forks source link

Stop throwing Exception in Tests cases #426

Closed sbouchet closed 1 year ago

sbouchet commented 1 year ago

from https://github.com/eclipse/lsp4mp/issues/424#issuecomment-1735102458, i suggest to stop throwing generic Exception from Test ( like https://github.com/eclipse/lsp4mp/blob/f191f42ec1a311937ecadb198c7498194be2dac3/microprofile.jdt/org.eclipse.lsp4mp.jdt.test/src/main/java/org/eclipse/lsp4mp/jdt/core/restclient/java/MicroProfileRestClientJavaCodeLensTest.java#L45 ). This swallow any potential errors/regression and let the test passes.

fbricon commented 1 year ago

test don't pass if an exception is thrown. Exceptions are not swallowed by the Junit runner

sbouchet commented 1 year ago

currently, the Test expect to throw Exception. that's why #425 wasn't failing tests. ( see https://ci.eclipse.org/lsp4mp/job/lsp4mp/job/master/259/consoleFull ) lot of NPE , no skip errors. Tests method should never throws any Exception. If an exception is expected, there are several ways to test it, like described in https://github.com/junit-team/junit4/wiki/Exception-testing.

fbricon commented 1 year ago

No it's not testing an expected exception. The throws Exception statements are merely a lazy way a not bothering with catching potential unexpected exceptions.

If an unexpected exception was thrown, the test would be in error (not a failure)

https://github.com/eclipse/lsp4mp/pull/425 not failing tests, as it should have, was caused by the (benign?) exception being swallowed in a lower stack.

sbouchet commented 1 year ago

Well ok then, closing.