Testing Framework Limitations

[jake]

I have been diving into the testing code today and have found some issues. I think the first one may warrant a high but the second one is just a preference. I can commit code for both if that works for people.

First, several assertions in the AgaviViewTestCase are broken. In fact, nearly all the assertions that do anything with the response object share the same problem. The only time that the response object sets up the appropriate http headers is when the send method is called on the response object. This happens in the controllers dispatch method, which is never called. So, methods that are broken include:

• assertResponseHasContentType
• assertResponseHasHeader
• assertResponseHasCookie - Depending on whether view sets cookie

There may be others. I'm not sure what the 'right' way to fix this is. For now I have extended the AgaviViewTestCase and overridden runView with the following changes:

• Second parameter specifies a method (read, write, etc)
• dispatch the controller with send_response set to true
• Wrap the dispatch in an ob_start/ob_end_clean so we don't get output
• Call setResponse on the container with the return from the controller

This works for my purposes.

Second, the test framework requires that all test code exist under one top level test directory. This prevents you from creating a layout such as:

  app/modules/Default/tests/actions/LoginActionTest.php

Instead you are restricted to using something like:

  app/tests/Default/actions/LoginActionTest.php

Although this is how PHPUnit recommend the setup I don't think it should be enforced as a requirement. My suggestion would be to do one of the following.

1. Change line 108 in AgaviTesting.class.php such that it doesn't require a prefix. At least at a minimum, in this case, you can specify the full path to your tests in the suites.xml file.
2. Add an optional module attribute to the suite or testfiles element. If specified, the testfile becomes %core.module_dir%/%config.module_name%/tests/%testfile%. I prefer this greatly.

[felix]

I'll go look into the assertions, I'm pretty certain they did work at some point. I guess we need testcases for the testing subsystem. However, everything in the testing system is pretty alpha so a certain caution while using is advised.

I don't think that doing a full dispatch and capturing the output is the right way to solve this. That actually duplicates what the current flow-test does and it introduces a ton of new variables in the testing harness - there'd be the action, all action and global filters... The intended use for the view-test is to test a view, isolated from any other framework component (well, as far as that's possible).

The directory structure issue could best be solved by not messing with any absolute path. You could then specify %core.module_dir%/path/to/your/module/tests/TestFile.php as path. This solution is more flexible than allowing for a module parameter - it covers libraries with own tests as well as any other test location you'd prefer. It also requires less extra code to handle, so that's probably the best way to go.

[felix]

(In [4157]) testfiles with an absolute path are now left unprefixed.

refs #1113

[felix]

Hi Jake,

I haven't tested all assertions but the ones I tested work perfectly fine. I think the main problem is a misconception on how the viewtest works: The view-test assertions only test the view itself. So the assertion "assertResponseHasHeader" actually tests that the view under test sets the given response header. There's more ways to set headers in agavi (output type etc..) that are not taken into account by this assertion - that would be a flowtest.

I see that the assertions are named a bit misleading, so I'll close this ticket and create a new one stating that we should rename these asssertions to a better name - maybe something along the lines of "assertViewSetsHeader(foo)" which would make the scope of the assertion clearer.

I hope that clears it up for the moment.

felix

[jake]

The assertion that I had the biggest issue was the assertResponseHasContentType. If you test using a JSON view with assertResponseHasContentType('application/json; charset=UTF-8') (as defined in the default output_types.xml), which I would expect to work, it doesn't. It doesn't work for the reasons that you stated. It may be that that assertion actually needs to be moved to the flow test since you actually don't want your views having to do setContentType.

[felix]

The assertion does not work because the content type in the output_types.xml is part of the global response, not of the view's respone object so that would be a flow-test.

I would not move the assertion - there has to be a way to assert that the view *explicitly* sets the content type. So it's fine where it is.

[jake]

Then is there a place for the exact same assertion in a flow test?

[felix]

Yes, except that it works on the global response. If you plan on working with flow tests you should have a look at the latest changes in trunk, there has been some major refactoring on the flowtest.