Implemented: Move script functionality into TWiki::UI module
Description
The attached zip files contains a patch that removes the bulk of the functionality from most of the bin scripts and places it in a set of modules under TWiki/UI instead. The rationale behind restructuring the scripts in this way is as follows:
- We are able to call the scripts from the command line
- We can call functions that used to be hidden in the scripts from other lib modules
- It is significantly easier to understand the code
I have benchmarked the two implementations several times. On my server, Apache 1.3 localhost, averaged over 10,000 topic accesses, I get the following numbers:
Contents of the zip
Prerequisites: TWiki CVS as of 8-Apr-2004, Test::Unit, wget
The zip file contains replacements for most of the scripts in bin, and adds two new directories, tools/test/script_tests and lib/TWiki/UI. There is also a new perl module, TWiki/UI.pm, that is only used by modules in TWiki/UI (at present).
Test description
Also included in the zip is a test framework and test set for refactoring changes.
The test suite
CGIScriptsSuite.pm
is a
Test::Unit
suite designed to exercise the
CGI scripts in TWiki, and is intended primarily to support refactorings. It does this by comparing script results from one server with results from the same script on another, on the assumption that the two servers are standard TWiki installations and both have identical data and pub areas (they can even share the same data and pub if necessary).
The general idea is that you create two CVS checkout areas, keeping one on the latest proven code and the other on bleeding edge code. Running the suite will tell you what you have changed in the system from a user perspective. Ultimately there should be an automatic script that maintains these areas, runs the tests, and mails the results to the core team.
CGIScriptSuite uses 'wget' extensively, so it's generally wise to use
http://localhost
as your server.
Note that as of April 2004 the script tests are very naive; many of them are little more than compile and compare tests. All the scripts need more sophisticated testing of their various parameters. Even in this naive form they have already picked up a number of errors.
The
CGIScriptSuite is run with the command:
perl TestRunner.pl CGIScriptSuite.pm
Individual tests can be run by using the name of the test e.g.
perl TestRunner.pl viewScriptTest.pm
WARNING if tests fail, there is a chance that parts of the fixture may be left. You can make sure you delete any fixture leftovers by:
rm -f <path to data>/Sandbox/AutoCreated*.*
rm -rf <path to pub>/Sandbox/AutoCreated*.*
for both old and new installations. Please don't complain at me until you've tried this and re-run the tests.
Test Status
The following tests are known to FAIL in some respect:
- viewfile - fails in old code as well
- save - fails in old code as well
- upload - fails in old code as well
In each case these failures are due to script errors that existed before the refactorings - and therefore the tests - were done. AFAICT they are genuine errors.
It will take time before all the tests pass reliably.
--
CrawfordCurrie - 10 Apr 2004
Thanks Crawford, this is a good step into the right direction, making it easier to integrate new functionality.
--
PeterThoeny - 11 Apr 2004
Great, I look forward to its rapid inclusion into the codebase.
--
MartinCleaver - 11 Apr 2004
Nice work Crawford
It is now in
TWikiAlphaRelease and installed at TWiki.org
The following scripts have not changed: geturl, installpasswd, mailnotify, passwd, register, testenv. geturl and testenv are out of scope, but the other ones could be fixed as well.
--
PeterThoeny - 11 Apr 2004
Commendable!
--
MartinCleaver - 11 Apr 2004
Installation issues discussion deleted
Thanks for your time, Crawford. Things work again.
--
ArthurClemens - 11 Apr 2004
I noticed a small issue possibly related to this patch:
WebStatistics does not get updated anymore on TWiki.org. When I update it on the topic, the user is
WebHome instead of
TWikiGuest (or guest as it incorrectly was)
--
PeterThoeny - 12 Apr 2004
Wierd - the user is
TWikiGuest on my installation. The command-line interface has changed, but only to add a -logdate parameter. I see a couple of things it
might be - I'll get back. --
CC
Argh, this is painful. Rather than building a patch, I'll describe the fix here:
Line 45 of Statistics.pm: change
my $logDate = $query->param( 'logdate' );
to
my $logDate = $query->param( 'logdate' ) || "";
Line 66 of statistics; insert a line:
my $theUrl = $query->url;
-- CC
'Patch' applied to CVS.
--
ArthurClemens - 12 Apr 2004
There are some more issues with the statistics. Glancing over the code, the logic has changed:
- TWiki::basicInitialize() is not called if in shell mode
- logdate parameter gets lost if in shell mode
- TWiki::initialize is called one time too much when in cgi mode
- TWiki::UI::Statistics::statistics calls accesses $query regardless if in cgi mode or not
To keep the quality of TWiki high, please make code changes carefully and/or test them out when submitting patches.
I would fix it myself, but running out of time...
--
PeterThoeny - 13 Apr 2004
Ping on the statistics bugs. I intend to create a new
TWikiBetaRelease but would like have these bugs fixed first.
--
PeterThoeny - 20 Apr 2004
I'm sorry, you've completely lost me. Are you saying there are bugs in the statistics reporting that were introduced by these changes? If so, what are the conditions under which the bugs occur? Re: your glance over the code:
- TWiki::basicInitialize() is not called if in shell mode
- No, it doesn't get called in CGI mode. This is because CGI mode uses a full TWiki::initialize, which calls basicInitialize anyway.
- logdate parameter gets lost if in shell mode.
- Gets lost? I don't understand this; the logdate parameter (which I added) is placed into the CGI query and passed in to the UI/Upload.
- You are correct, I was confused by the fact that the CGI query is set in shell mode (removed now) -- PTh
- TWiki::initialize is called one time too much when in cgi mode
- No, it is called on each web, with different parameters each time. This is the same as the old statistics script.
- Plus called one more time at the beginning -- PTh
- TWiki::UI::Statistics::statistics calls accesses $query regardless if in cgi mode or not
- Correct. The shell mode uses new CGI("") to create a query to simplify passing of parameters and make it consistent with the other scripts. This was a deliberate design decision.
- I was confused by using a cgi query in shell mode
I am unable to find any problem with the statistics generation on my test site in any respects, though I am continuing to search. I can't see any problem with the stats on TWiki.org either, which I believe is running the same code?
--
CrawfordCurrie - 20 Apr 2004
Isn't this in CVS now? Moving to
PatchAccepted.
--
WalterMundt - 25 Apr 2004
Fixed these bugs in the statistics script.
- Remote user was not honored when called in cgi mode; user
Main.WebHome
was logged when you force executed the script in WebStatistics
- Removed double-init when called in cgi mode
- User guest was always shown in progress page instead of remembered user (old bug)
- All files and subdirs in the data dir have been taken as webs, producing many errors when generating stats for all webs (old bug)
-
entities have been shown when called in shell mode (old bug)
Fixes are in
TWikiAlphaRelease and at TWiki.org.
--
PeterThoeny - 04 May 2004