Fixed: Simpletest FilterEditorLinkValidateTestCase silently broken on PHP 5.3

[indigoxela]

It might have been broken for a while, but nobody realized because it doesn't produce an actual test failure.

Only on PHP 5.3:

PHP Parse error:  syntax error, unexpected ')' in /home/runner/work/backdrop/backdrop/core/modules/filter/tests/filter_dialog.test on line 26
ERROR: FATAL FilterEditorLinkValidateTestCase: test runner returned a non-zero error code (255).

When reading "syntax error" in 5.3 my first thought is the short array syntax, but there doesn't seem to be any (we fixed one recently).

Here's an example: https://github.com/backdrop/backdrop/runs/3585091100?check_suite_focus=true#step:9:287

Note: no test failure.

[indigoxela]

Subtle, but sort of obvious - the trailing comma breaks it. It's inside a function, not an array definition, so PHP versions before 7.3 choke on it.

# /usr/bin/php5.6 -l core/modules/filter/tests/filter_dialog.test
PHP Parse error:  syntax error, unexpected ')' in core/modules/filter/tests/filter_dialog.test on line 26
Errors parsing core/modules/filter/tests/filter_dialog.test

[klonos]

We seem to be inconsistently using either multiple string parameters, or an array within parent::setUp(). So we have these variations:

1. parent::setUp('module'); (makes sense I guess, when there's a single module)
2. parent::setUp(array('module'));
3. parent::setUp('module1', 'module2', 'module3');
4. parent::setUp(array('module1', 'module2', 'module3'));

In the case of multiple modules, people will be splitting them in separate lines, for readability. So you'll have this, which I think should work w/o any problems:

  parent::setUp(
    array(
      'module1',
      'module2',
      'module3',
    )
  );

But also this, which will break things:

  parent::setUp(
    'module1',
    'module2',
    'module3',
  );

Since people may be copy/pasting code and then tweaking it, I'm wondering whether it would be a good idea to establish a coding standard where we require the array variant when multiple modules (less chances to break things).

[quicksketch]

Thanks folks!

@klonos the different arguments for setUp() is problematic for extending test classes and it would be great if we could only support the first syntax. But that should be a separate issue from this particular problem which is a straight syntax problem.

I went ahead and merged backdrop/backdrop#3739 into 1.x and 1.19.x. Thanks!