Closed Bug 1137674 Opened 8 years ago Closed 8 years ago

Merge t/003safesys.t with t/001compile.t

Categories

(Bugzilla :: Testing Suite, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
Bugzilla 6.0

People

(Reporter: LpSolit, Assigned: LpSolit)

References

Details

Attachments

(1 file)

Both t/001compile.t and t/003safesys.t are doing *exactly* the same thing: loop over all files in @testitems and compile them, 001 to detect if the file compiles correctly, and 003 to detect if it uses system() and exec() correctly. In both cases must the file be compiled, the only difference being that 001 looks for "syntax OK" while 003 looks for "arguments for Support::Systemexec::(system|exec)" in the returned value. This is a waste of time.

Moreover, 001compile.t is much more clever than 003safesys.t to determine which files to compile or not, based on installed Perl modules (optional features). Also, 001compile.t supports $ENV{PERL5LIB}. 003safesys.t doesn't. So we should kill 003safesys.t and let 001compile.t do all the compilation work. This would automatically fix bug 1116118 on master.
Attached patch patch, v1Splinter Review
Attachment #8570454 - Flags: review?(dylan)
In case you are wondering, 001compile.t has a shortcut for .pm files: it calls use_ok() instead of invoking the perl interpreter, because this is much faster. The downside is that .pm files are not loaded with -MSupport::Systemexec, i.e. 001compile.t doesn't check the number of arguments passed to system() and exec() in them. But this is not really a regression, because 003safesys.t is unable to detect system() and exec() in .pm files anyway. Said differently, 003safesys.t only works as designed with .cgi and .pl scripts. This is why I left the shortcut in 001compile.t.

13 years that these tests are broken, they never worked, and nobody ever noticed it. /me blames zach (bug 97976 comment 11). ;)
Comment on attachment 8570454 [details] [diff] [review]
patch, v1

Review of attachment 8570454 [details] [diff] [review]:
-----------------------------------------------------------------

r=dylan
yes, kill it. kill it with fire.

// technically, if you wanted to do the Systemexec hack (inject symbols into every namespace) we can do that. I don't think we should do that though.
Attachment #8570454 - Flags: review?(dylan) → review+
Flags: approval?
Flags: approval? → approval+
To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   2ccf81d..56d130c  master -> master
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.