FacesRequest gives false positives in unit tests

[jervin]

I am new to Seam development, so forgive me if some of what I have to say betrays my newbie status.  I was digging through some of the unit test code in a project at the company I am currently working at and I uncovered something rather unfortunate that the tests were running and, in reality should fail, but they were erroneously passed.  The tests used the common pattern of the BaseSeamTest.FacesRequest class, overriding the methods as needed and placing the assertions in the renderResponse() method.  In fact, not only is this pattern common in the codebase at my company, but is common when I was googling for examples on the web.

example:
@Test
public void testingSomething() {
  new FacesRequest() {

    public void renderResponse() {
      assert( ... );
    }

  }.run();
}
The problem was that sometime before, functionality was added that forced a login before accessing the view being tested.  This meant that the renderResponse() method was never called during the test, really invalidating it.  In fact, it was not benign in this instance, the assertions should have failed.

So my question is, is there someway to work around this?  I know I could add some boolean flag manually to check this:

@Test
public void testingSomething() {
  final boolean[] assertionsInvoked = new boolean [] { false };
  new FacesRequest() {

    public void renderResponse() {
      assertionsInvoked[ 0 ] = true;
      assert( ... );
    }

  }.run();
  assertTrue( assertionsInvoked[ 0 ] );
}

This is tedious and it is likely that developers (I should know since I am one) would forget to do this.

The second approach would be to try and enhance SeamTest and add a check so that the renderResponse method (almost all assertions seem to wind up there), is actually invoked during the test.  The problem with this is that the SeamTest framework is not easily modified, since FacesRequest is an inner non-static class of BaseSeamTest.  This prevented the use of a simple wrapper class and the user of Java utility libraries like CGLib and org.springframework.aop.  So the solution I have been pursuing is the use of Aspects that will intercept calls to both run() and renderResponse() and ensure that they are invoked.

Still this leaves me with a sinking feeling, as a Software Dev I love the fact that I got the chance to try a bunch of new libraries and finally got a crack at Aspects, but I wonder if something simpler/better does not exist.

Any ideas?

Thanks,
James E. Ervin

[jeanluc]

funny, I just wanted to post a similar message as today I ran into the very same issue. Slightly different scenario: with Seam-managed transactions and container-managed persistence contexts, the transaction that encompasses phases up to INVOKE APPLICATION was rolled back (due to a Hibernate validator exception). However, it was rolled back not during invokeApplication() but a bit later, when Seam's PhaseListener decided to commit the transaction. As a result, the invokeApplication() method finished successfully. renderResponse() was never called and the test succeeded.  A runtime exception appeared in the log, but the test (ran with TestNG) did not fail.

I looked in AbstractSeamTest but didn't find any place which I can easily override and check. I haven't had time to dig deeper; something else than AOP to try is to create a fixed version of AbstractSeamTest and inherit from it.

[kapitanpetko]

I don't have a solution, but one more thing to watch out for is that renderResponse is not called if invokeApplication causes a
redirect. You should have your asserts in another FacesRequest or in invokeApplication (if that makes sense).

invokeApplication should probably return something to warn you that renderResponse will not be executed. But that is not a perfect
solution either...

You can look into AbstractSeamTest.emulateJsfLifecyle to see how this all works. Unfortunately it is private, so no easy way
to override it.

[jeanluc]

Luckily, there's nothing inside AbstractSeamTest that requires it to be in a Seam package so it's possible to create another version with whatever extra functionality is needed and inherit from it.

[jervin]

You know I had not considered that possibility, that would be simpler than introducing AOP into the project just for this annoyance.

I had thought of adding checks to see that overridden methods would be run in the unit test or adding an annotation that my new implementation could check.

The first idea is that if you bother to override the appropriate FacesRequest method, it probably means you mean for it to be run during the test.  In the following example, since renderResponse has been overridden, a check during the run method to see that if the subclass overrode it would mean that the user wants for it to be invoked during that invocation of run, otherwise throw the assertion error.

@Test
public void testingSomething() {
  new FacesRequest() {

    public void renderResponse() {
      assert( ... );
    }

  }.run();
}

The second idea would be to use an annotation on the overridden FacesRequest method to do the same.  I just made up the annotation @Invoke to signify it better be run during the invocation of run otherwise an assertion error would be thrown.

@Test
public void testingSomething() {
  new FacesRequest() {
    @Invoke
    public void renderResponse() {
      assert( ... );
    }

  }.run();
}

A third idea would be to use an annotation to specify that you didn't care that an overridden method was invoked.

All of these ideas would require AOP, but with reimplementing the AbstractSeamTest class at least I could use the Spring Framework AOP, which a very nice wrapper to do the AOP at runtime, so there would be no need to introduce the AOP builder or compilation.  I will probably try it sometime early next week.  If I am successful, I think I may feel a little blog entry coming on :)

[jonbullock]

Has anyone come up with a solution to this as we're having a similar problem in Seam 2.2.2-Final??