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

Status: RESOLVED FIXED

(Bugzilla :: Testing Suite, enhancement)

Description

[Frédéric Buclin]

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.

Comment 1

[Frédéric Buclin]

Attachment: patch, v1

Comment 2

[Frédéric Buclin]

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 3

[Dylan Hardison [:dylan] (he/him)]

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.

Comment 4

[Frédéric Buclin]

To ssh://[e-mail]/bugzilla/bugzilla.git
   2ccf81d..56d130c  master -> master