Closed
Bug 165756
Opened 22 years ago
Closed 22 years ago
Running tests without checksetup causes failure
Categories
(Bugzilla :: Testing Suite, defect, P1)
Tracking
()
RESOLVED
FIXED
Bugzilla 2.18
People
(Reporter: jacob, Assigned: bbaetz)
References
Details
Attachments
(1 file)
(deleted),
patch
|
bugreport
:
review+
jacob
:
review+
|
Details | Diff | Splinter Review |
If one (such as myself :) were to checkout Bugzilla and then immediately run the
tests, 001compile.t will fail 3 times because Config.pm is looking for the
shutdownhtml param which won't exist in a non-installed directory.
Assignee | ||
Comment 1•22 years ago
|
||
Grrrrrr.
Whats happening is that the 'require' of CGI.pl is checking the shutdownhtml
param, but this happens before the Bugzilla::Config init code runs.
I need to swap orderings arround a bit; this is only temporary until
Bugzilla::CGI happens, anbd the check moves there.
Assignee | ||
Comment 2•22 years ago
|
||
Actually, no - I was wrong.
This is, in fact, WONTFIX
checksetup.pl needs to do updaing of old params, which means that we can't load
new ones first. If we can't load new ones, then we can't cope with the file not
existing.
Lets just fix checksetuptorun w/o problems, and then update tbox.
(Bugzilla::CGI will move the shutdownhtml check to creating a new CGI object,
but it will still need to acess the Maintainer for Bugzilla::CGI errormessages
which must happen at BEGIN time)
Depends on: 123957
Assignee | ||
Comment 3•22 years ago
|
||
Now that bug 123957 is fixed, tinderboxes should run checksetup before running
tests.
Summary: Running tests w/out checksetup.pl causes Config.pm to fail → tinderboxes need to run checksetup before tests
Reporter | ||
Comment 4•22 years ago
|
||
To me, this is backwards... even when installing. I'm gonna wanna run the tests
to make sure everything is working before I install it (ie, run checksetup.pl).
For this purpose, we should probably have a .t file that checks for the
existence of the required modules (but that's another bug :)
Assignee | ||
Comment 5•22 years ago
|
||
Our tests aren't functional tests - the presense of lack of a spelling mistake
will not affect the results.
Once we have functional tests, then a database will be required to run them...
It is, of course, possiblew that we could have checksetup create/run on a test
db (and we probably will) but for teh moment....
Comment 6•22 years ago
|
||
Can't we configure it to only fail on a GetParam? Why is -c running code anyway?
Priority: -- → P1
Target Milestone: --- → Bugzilla 2.18
Assignee | ||
Comment 7•22 years ago
|
||
-c runs code because -c runs BEGIN blocks, and |use| are effectivly BEGIN blocks.
ACtually, I just looked at this again, and I have a fix.
The problem is that packages should not be requiring globals.pl/CGI.pl themselves.
This is really bad if they do so after the package line (because stuff is then
in the wrong namespace), but its not a good idea anyway.
Fix coming.
Assignee: zach → bbaetz
Summary: tinderboxes need to run checksetup before tests → Running tests without checksetup causes failure
Assignee | ||
Comment 8•22 years ago
|
||
Comment 9•22 years ago
|
||
Comment on attachment 97728 [details] [diff] [review]
patch
r=joel
Would like a 2xr from someone more expert on the implications of style
decisions.
Attachment #97728 -
Flags: review+
Reporter | ||
Comment 10•22 years ago
|
||
Comment on attachment 97728 [details] [diff] [review]
patch
I'm not an expert, but I can't see any problems with this (as long as each
caller requires CGI.pl/globals.pl) before the .pm's...
r2=jake
Attachment #97728 -
Flags: review+
Assignee | ||
Comment 11•22 years ago
|
||
Checked in.
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Updated•12 years ago
|
QA Contact: matty_is_a_geek → default-qa
You need to log in
before you can comment on or make changes to this bug.
Description
•