1137669 - 003safesys.t doesn't test any file due to a missing -T argument (and broken syntax in Support::Systemexec)

Status: RESOLVED FIXED

(Bugzilla :: Testing Suite, defect)

Description

[Frédéric Buclin]

While investigating bug 1116118 comment 7, I found why 003safesys.t is behaving strangely in master. There are 2 bugs:

- scripts are invoked without the -T argument, making all scripts to be skipped. This bug exists since 003safesys.t exists, i.e. since Bugzilla 2.16, see bug 97976.

- since Bugzilla 5.0, we require Perl 5.10.1 and now t/Support/Systemexec.pm syntax is no longer valid, because bug 996893 added |use strict| to it, and this module was very badly written. This makes all tests to abort too.


We should fix these problems before gerv starts optimizing a broken 003safesys.t script. :)

Comment 1

[Frédéric Buclin]

Attachment: patch for 5.x, v1

Comment 2

[Frédéric Buclin]

Attachment: patch for 4.x, v1

This patch is for Bugzilla 4.x. It doesn't require t/Support/Systemexec.pm to be fixed as we don't enforce strictness in 4.x.

Comment 3

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

Comment on attachment 8570443 [details] [diff] [review]
patch for 5.x, v1

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

r=dylan

I hope to replace this test with a sufficiently decent Perl Critic setup. This is the third weirdest abuse of subroutine prototypes I have ever seen.

Comment 4

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

Comment on attachment 8570447 [details] [diff] [review]
patch for 4.x, v1

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

r=dylan

Comment 5

[Frédéric Buclin]

As 003safesys.t is about security, I think it should land in 4.0 and 4.2 too.

Comment 6

[Frédéric Buclin]

This patch is not needed on master as 003safesys.t has been merged with 001compile.t, see bug 1137674.

Comment 7

[Frédéric Buclin]

Actually, for master, I just need changes made in t/Support/Systemexec.pm.

Comment 8

[:glob ✱]

(In reply to Frédéric Buclin from comment #5)
> As 003safesys.t is about security, I think it should land in 4.0 and 4.2 too.

sure, but there's no need to cut a release specifically for this; it can ride along with the next security release.

Comment 9

[Frédéric Buclin]

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

To ssh://[e-mail]/bugzilla/bugzilla.git
   c26e64e..f5fb20f  5.0 -> 5.0

To ssh://[e-mail]/bugzilla/bugzilla.git
   f0c2b6b..fc4a6dd  4.4 -> 4.4

To ssh://[e-mail]/bugzilla/bugzilla.git
   44a9c6d..6cfdba0  4.2 -> 4.2

To ssh://[e-mail]/bugzilla/bugzilla.git
   d5f0eb4..3961356  4.0 -> 4.0