PrintingFunctionsTrait::is_printing_function(): add tests

[rodrigoprimo]

Description

In preparation for supporting PHPCS 4.0, this PR adds unit tests for the PrintingFunctionsTrait::is_printing_function() method.

Suggested changelog entry

N/A

[jrfnl]

❌ I'm going to block this PR in its current state.

The is_printing_function() can't be considered properly tested unless the get_printing_functions() method is also tested.
So either dedicated tests for the get_printing_functions() method need to be added ahead of adding the tests for the is_printing_function() method; or the is_printing_function() tests need to be expanded to include covering what happens in get_printing_functions() (which I believe would be the wrong choice).

Adding tests only for is_printing_function() without having tests for get_printing_functions() is misleading and incomplete.

---

As a side-note regarding the test setup:

I understand what you are trying to do, but why did you choose this way to do it ?

I mean, why not just use PrintingFunctionsTrait; in the test class ? (which would also give the tests access to the private properties if needed).

[rodrigoprimo]

Thanks for the review, Juliette.

The is_printing_function() can't be considered properly tested unless the get_printing_functions() method is also tested.

Regarding testing get_printing_functions(), I opted to add tests only for is_printing_function() because I wrote those tests with the PHPCS 4.0 changes in mind and thought that tests for get_printing_functions() could be added later. I was trying to add just the tests required for PHPCS 4.0 to keep the scope focused on this update. That said, I agree that get_printing_functions() should also have tests, and I can add them to this PR if this is the approach that you prefer.

I understand what you are trying to do, but why did you choose this way to do it ?
I mean, why not just use PrintingFunctionsTrait; in the test class ? (which would also give the tests access to the private properties if needed).

Regarding the test setup, I opted for the anonymous class approach to keep a clear separation between the test subject and the test class. Using use PrintingFunctionsTrait; directly in the test class means the test class becomes the thing it's testing. For a simple trait like PrintingFunctionsTrait, this is not a big concern, but for a trait like WPDBTrait it starts to feel less clean. I wanted to keep a consistent approach across all trait tests.

I can see that the anonymous class approach adds boilerplate (and in the case of the WPDBTrait, I opted to create a helper class that uses the trait instead of an anonymous class, which adds even more boilerplate). I'm more inclined to favor the separation between the test subject and the test class and keep the current structure. That said, I'm open to switching to use in the test class if you think the simplicity outweighs the separation concern. What is your preference?

[jrfnl]

The is_printing_function() can't be considered properly tested unless the get_printing_functions() method is also tested.

That said, I agree that get_printing_functions() should also have tests, and I can add them to this PR if this is the approach that you prefer.

I already said that this PR is blocked until tests for get_printing_functions() are added. Either in a separate PR or in a separate commit in this PR.

Regarding the test setup, I opted for the anonymous class approach to keep a clear separation between the test subject and the test class. ... I wanted to keep a consistent approach across all trait tests.

Understood. Thanks for sharing your reasoning.

I can see that the anonymous class approach adds boilerplate. ... I'm more inclined to favor the separation between the test subject and the test class and keep the current structure. That said, I'm open to switching to use in the test class if you think the simplicity outweighs the separation concern. What is your preference?

No strong preference. I totally see the value of the separation, but I can also see the value of the simpler approach for a simple trait like this if it would make testing the trait more straight-forward (less jumping through hoops to test what needs testing).

I guess for a first set of tests, I would be pragmatic (whatever works), while when iterating on it after, I would consolidate to a standardized, consistent approach based on what we then know is needed. It's difficult to standardize/make things consistent if it's not clear yet what will be needed across the board.

Hope that input helps.