Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 355194

Summary:

Server Test framework: test throwing exception not always marked as failed

Product:

z_Archived

Reporter:

Otmar Caduff <otmar.caduff>

Component:

Scout

Assignee:

Jeremie Bresson <jeremie.bresson>

Status:

CLOSED FIXED

QA Contact:

Severity:

normal

Priority:

CC:

fwi, jeremie.bresson, ken.lee, mvi, trekking09, zimmermann

Version:

unspecified

Flags:

trekking09: juno+
trekking09: kepler+

Target Milestone:

---

Hardware:

OS:

Windows 7

Whiteboard:

Bug Depends on:

402308

Bug Blocks:

Attachments:

Description	Flags
Proposed patch.	none
Alternative Patch	none
Alternative Patch	none

Description Otmar Caduff

2011-08-19 05:07:07 EDT

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

Comment 1 Otmar Caduff

2011-08-19 05:07:29 EDT

Created attachment 201769 [details]
Proposed patch.

Comment 2 Jeremie Bresson

2013-03-04 06:45:09 EST

Added in the Testing Support container.

Comment 3 Adrian Moser

2013-08-15 09:06:40 EDT

Created attachment 234455 [details]
Alternative Patch

Alternative, simpler patch which should have the same effect.

Comment 4 Adrian Moser

2013-08-15 09:11:23 EDT

Applied to Scout branch 3.8.

Will be merged by Ken or Stephan to Scout branch 3.9.

Comment 5 Adrian Moser

2013-08-15 09:17:26 EDT

Created attachment 234456 [details]
Alternative Patch

Comment 6 Ken Lee

2013-08-20 19:23:53 EDT

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

Comment 7 Ken Lee

2013-08-20 19:24:16 EDT

@Jérémie: Please verify.

Comment 8 Jeremie Bresson

2013-08-22 07:39:47 EDT

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.

Comment 9 Jeremie Bresson

2013-09-16 04:18:38 EDT

@Adrian Moser:
As discussed, I have started a change for you:
https://git.eclipse.org/r/16455

Comment 10 Adrian Moser

2014-03-18 10:47:42 EDT

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).

Comment 11 Matthias Zimmermann

2014-07-01 03:19:22 EDT

Shipped with Eclipse Luna Release