Re: [PATCH] Add test suits for servlets

["Ilya A. Volynets-Evenbakh" <ilya@total-knowledge.com>]

See comments below

Sergey wrote:
> From: Sergey Jukov <sergey@total-knowledge.com>
>
> +Sergey Jukov <sergey@total-knowledge.com>            Mon, 25 Sep 2006 13:30:00 -0800
> +- Add testing suits for servlets
>   
There is _one_ test suit, and it's not for servlets, it's for CPPSERV
functions.
At least that's what I'm looking for. So, ChangeLog entry should list
specific
functions that are being tested.
> --- a/Makefile.adon
> +++ b/Makefile.adon
> @@ -1,2 +1,5 @@
>  ADON_SUBDIRS := csp-store hello attributes reset utils fileupload redirect cookies session headers sharedata params BooksODBC database index
>  EXTRA_DIST := README engine.xml
> +the_tests := tests/sessionGetSession.pl tests/sessionGetNumberHits.pl tests/sessionGetSessionId.pl tests/sessionGetCreationTime.pl tests/sessionGetAccessedTime.pl tests/sessionGetMaxTime.pl
> +TESTS := $(the_tests)
> +check_SCRIPTS:=$(TESTS)
>   
You are making this far more complex then it should be. We don't have
conditional tests, or any
other crap like that. You should have _one_ line. Finally, you should
not be listing tests in top-level
directory. Although it does work in this particular case, it is not
supported an is not guaranteed
to work in future.
> \ No newline at end of file
>   
Ugh.

> diff --git a/tests/sessionGetAccessedTime.pl b/tests/sessionGetAccessedTime.pl
> new file mode 100755
> index 0000000..17c0dda
> --- /dev/null
> +++ b/tests/sessionGetAccessedTime.pl
> @@ -0,0 +1,28 @@
> +#!/usr/bin/perl
> +
> +use LWP::UserAgent;
> +$ua = LWP::UserAgent->new;
> +
> +my $req = HTTP::Request->new(POST => 'http://localhost/~sergey/csp/SessionServlet');
>   
Bad bad Sergey ;-)
Hardwiring paths will not make it very portable ;-)
> +$req->content_type('application/x-www-form-urlencoded');
> +$req->content('key=value');
> +
> +my $res = $ua->request($req);
> +
> +if ($res->is_success) {
> +	my $line = $res->content;
> +	$line =~ s/\n+//g;
> +	if($line =~ /Last Accessed time:\s(.*)Maximum/) {
> +		print "Last Accessed time: ".$1."\n";
> +	}
> +} else {
> +	print $res->status_line, "\n";
> +}
>   
I don't see
    1. Real test for failure
    2. Program exiting with non-zero code in case of failure.
> +
> +
> +
> +
> +
> +
> +
>   
Can we get rid of all these empty lines?

Same comments apply to rest of tests. Basically - I don't see
actual failure tests. I don't see corner-case tests. etc.

-- 
Ilya A. Volynets-Evenbakh
Total Knowledge. CTO
http://www.total-knowledge.com