Code review comment for lp:~felmas/mvhub/structure_fix

Revision history for this message
Dan MacNeil (omacneil) wrote :

lib-mvhub/t/Structure/display_structure_menu.t

lib-mvhub/t/Structure/process_synonym_form.t

should probably move to

  app-mvhub/t/mech/admin

They are more integration tests (test whole app like the user) than they are unit tests (test each sub separately )

These tests will not effect test coverage because the code isn't being instrumented ( modified by Devel::Cover to measure coverage) , the code runs on the webserver, which could be in Siberia

It should not be hard to get test coverage

both routines take a CGI object as a parameter. It is pretty straight forward to dummy up a CGI object to pass in. see:

lib-mvhub/t/Utils/clean_cgi_params.t

The problem is that both routines send to stdout instead of returning a value (ugly I know)

Two options:
 a. take as given that code is tested by integration tests

b. redirect stdout to a variable and then do pattern matching on that variable to check if the expected data is there.

http://perldoc.perl.org/functions/open.html

http://www.nntp.perl.org/group/perl.beginners/2007/05/msg91387.html

review: Needs Fixing

« Back to merge proposal