Community
Participate
Working Groups
Build Identifier: latest on branch 2011-Jun ScoutAssert.jobSuccessfullyCompleted(job) does not necessarily detect all exceptions, because it only checks the information available on the job object. For example, if a ProcessingException without a nested cause exception is thrown from within a test method, this should clearly mark the test as failed. However, in that case, that information is not available on the job object. Reproducible: Always Steps to Reproduce: 1. create a server test case 2. create a test method that throws a ProcessingException, providing no causing exception 3. the test will be considered as successfull by JUnit
Created attachment 201769 [details] Proposed patch.
Added in the Testing Support container.
Created attachment 234455 [details] Alternative Patch Alternative, simpler patch which should have the same effect.
Applied to Scout branch 3.8. Will be merged by Ken or Stephan to Scout branch 3.9.
Created attachment 234456 [details] Alternative Patch
The fix has been applied to the class org.eclipse.scout.rt.testing.commons.ScoutAssert (instead of ScoutAssert in the shared package) due to the testing framework refactoring in Scout 3.9 and Scout 3.10. Scout 3.9: Pushed to Gerrit: https://git.eclipse.org/r/#/c/15675/ Merged into release/3.9.1 branch with commit http://git.eclipse.org/c/scout/org.eclipse.scout.rt.git/commit/?id=a50dbe2c4c7d1f5c90574183cbb896e781988198 Scout 3.10: Pushed to Gerrit: https://git.eclipse.org/r/#/c/15672/ Merged into develop branch with commit http://git.eclipse.org/c/scout/org.eclipse.scout.rt.git/commit/?id=03f453bac2bba02737b2f2f0ef6b8121ddfdfc6b
@Jérémie: Please verify.
This is not OK in my opinion. Consider this store function (DesktopProcessService): @Override public DesktopFormData store(DesktopFormData formData) throws ProcessingException { //Business logic on the server (VetoException on error): if (formData.getName().getValue() == null || formData.getName().getValue().toLowerCase().matches("x+")) { throw new VetoException(TEXTS.get("ThisIsNotAValidName")); } //Rest of the persistence code //... } And this test: @RunWith(ScoutServerTestRunner.class) public class DesktopProcessServiceTest { /** * Test method for {@link DesktopProcessService#store(DesktopFormData)}. * The name is invalid. An error is expected from the Service. */ @Test(expected = VetoException.class) public void testStoreInvalidName() throws Exception { DesktopFormData formData = new DesktopFormData(); formData.getName().setValue("xxx"); SERVICES.getService(DesktopProcessService.class).store(formData); } } @Adrian: It seems to me that the patch that you have proposed (attachment 234456 [details]) doesn't do the same as the original patch (attachment 201769 [details]). Using Assert.fail(..) is not a solution in my opinion. We need to throw the exception in order to let JUnit check the expected Exceptions. Additionally there should be an Unit Test for the modified method. (ScoutAssert#jobSuccessfullyCompleted(JobEx) in the current patch). It is important to secure the expected behavior with a unit test. @People on the CC List: Other opinions are more than welcome.
@Adrian Moser: As discussed, I have started a change for you: https://git.eclipse.org/r/16455
The original problem mentioned by Otmar was solved directly in ServerJob: https://bugs.eclipse.org/bugs/show_bug.cgi?id=421958 Therefore the proposed workaround by Otmar is not needed. I have added a new Unit test to prove the desired exception handling (see ScoutServerJobWrapperStatementTest).
Shipped with Eclipse Luna Release
