Simplify logic in unit tests

[fredden]

As suggested in #1586 (comment), using an array/foreach for a single value is inefficient. This pull request resolves the known cases of this.

[jrfnl]

@fredden Thanks for this PR.

When I originally saw it, I was a bit in two minds about it as - like I also said in the other ticket - the parameter for $lines being an array was intentional as it allowed more easily to test the logic of the sniff, not just the individual functions being flagged.

Having thought it over a little more, I'm happy to accept this change.
It simplifies the test files a little and if at some point in the future more tests specific to the logic of the sniff would be added, we can add an extra test method to specifically handle those tests.
(We should probably rethink the tests for these list-based sniffs anyway, but that's for later, the 10.0 release has priority and test changes can be made at a later stage).

I've left a few small nitpicks inline.

Other than that, I would like to ask you to remove the change for the RemovedExtensionsUnitTest file.
I may have mentioned this before, but that whole sniff will be removed in the near future - see #1022 - and making changes in the test file may conflict with the local WIP branch I have for this, so it'll make my life easier if that file could be left alone.

[fredden]

When I originally saw it, I was a bit in two minds about it [...] Having thought it over a little more, I'm happy to accept this change.

I should think so! As per the pull request description, this change was specifically requested by you here: #1586 (comment)

I've left a few small nitpicks inline.

Thanks for these. I'm surprised that the indentation problems weren't detected (and auto-fixed) by PHP_CodeSniffer. I most certainly ran both vendor/bin/phpcbf and vendor/bin/phpcs before committing the changes.

Other than that, I would like to ask you to remove the change for the RemovedExtensionsUnitTest file.

This would make the test-suite inconsistent, which defeats the purpose of this pull request. Perhaps we push pause on this pull request while that other work is in progress. I'm happy to resolve conflicts here.

[jrfnl]

When I originally saw it, I was a bit in two minds about it [...] Having thought it over a little more, I'm happy to accept this change.

I should think so! As per the pull request description, this change was specifically requested by you here: #1586 (comment)

Sorry, didn't mean to come across as dismissive. That comment also says "they should have a few of cases which do have multiple lines", which refers to the logic tests I was expecting, which appear to be missing.

I've left a few small nitpicks inline.

Thanks for these. I'm surprised that the indentation problems weren't detected (and auto-fixed) by PHP_CodeSniffer. I most certainly ran both vendor/bin/phpcbf and vendor/bin/phpcs before committing the changes.

Interesting. I'll have to have a look at why that wasn't flagged by PSR12 in that case, but that's for after the release.

Other than that, I would like to ask you to remove the change for the RemovedExtensionsUnitTest file.

This would make the test-suite inconsistent, which defeats the purpose of this pull request. Perhaps we push pause on this pull request while that other work is in progress. I'm happy to resolve conflicts here.

As I said: the file will be removed anyway, so I'm perfectly okay with accepting a little inconsistency until it actually is (removed).

I'd rather not leave this PR open as the New/Removed list based sniffs are frequently updated, so this PR would conflict very quickly and I'd prefer it not becoming a blocker for other PRs.

[jrfnl]

@fredden Just checking if you saw my last message. I'd like to get this merged ASAP without the RemovedExtensions file change.

If you worry about inconsistency - this creates inconsistency anyway as there are quite a few more test files, which use the same pattern which is now being removed in the files included in this PR. (New/RemovedClasses, New/RemovedIniDirectives etc)
I'm not too concerned about it.

[jrfnl]

I've removed the changes to the RemovedExtensions test file (with permission of @fredden). Once the build has finished I will merge this.

[jrfnl]

Failing build with Coveralls is a known issue (outside contributor) and is being worked on. Merging.