Test dependencies in the psr/log package??

[Rasmus Schultz]

I noticed the introduction of two PHPUnit-specific test-dependencies into the psr/log package:

https://github.com/php-fig/log/tree/1.1.0/Psr/Log/Test

How is this in any way acceptable?

The dependency isn't even declared in the "composer.json" file - but that's kind of besides the point.

First of all, PHPUnit isn't PHP - introducing facilities for a specific test-framework is extremely opinionated and does not belong in an package with interfaces designed to bridge projects and provide interoperability between different frameworks. (PHPUnit being a test-framework.)

Moreover, these facilities aren't a dependency, at all - you can use this package just fine without it. There is no rational reason not to package these dependencies separately.

These facilities belong in a different package that depends on psr/log, so you can version them separately. Stuffing these into the package itself is extremely controversial and could create a serious problem with versioning. (having to version the interfaces as 2.0 for a breaking change to the test-facilities!)

I'm calling for a bugfix release to remove these ASAP.

Who's even responsible for this?

There's no issue-tracker on the GitHub project, and I don't see any discussion about it in this forum.

wtf?

[Larry Garfield]

One of them has been there for years. The other was just added recently with
little notice.

This has been discussed before, and frankly I agree that the tests should not
be in the package at all. We have -util packages for this sort of thing.

Related discussion:

https://github.com/php-fig/log/pull/52#issuecomment-378323725

--Larry Garfield

[Alessandro Lai]

Sorry for adding this with so much little notice. I added that since the other one was already present, and that wouldn't change the number of implicit dependency on that package.

Obviously not using that class will not trigger any issue, but removing that now would be a BC, so I'm against doing that: breaking SemVer would be a very bad example here.

I'm all for splitting it into a separate package, but I would like the CC input on this, since my previous action seems to be debatable. In any case, this would require a 2.0 of psr/log, but I fear that that change would scare a lot of people... Even the 1.1 ruffed some feathers!

[Rasmus Schultz]

As I've noted on the PR thread:

Introducing the test facilities in the first place was a "bug", and fixing it will break tests only.

Versioning that fix as a major is wrong, since there are no breaking changes to the library itself.

If you're very concerned about breaking somebody's tests, you could do a minor release some months earlier with deprecations for these facilities and a note stating the date by which they will be removed.

Versioning the removal as a major will create an endless cascade of dependency issues for something that doesn't affect production code.

Please no.

--
You received this message because you are subscribed to a topic in the Google Groups "PHP Framework Interoperability Group" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/php-fig/DsmK600GAas/unsubscribe.
To unsubscribe from this group and all its topics, send an email to php-fig+u...@googlegroups.com.
To post to this group, send email to php...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/ed94acf6-698c-4410-8b10-44dba2f40ef7%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Chuck Burgess]

At first glance, I think I'm good with moving this out to a log-util package, with perhaps a bump to v1.2.  Adjusting testcases has never really urged me into being concerned about versioning, at least when working in my packages.  The BC extent of anyone actually depending on these for their own tests should just be a `require` section adjustment in their `composer.json`, right?  Namespacing of our other util packages that I spot-checked look like they stick to the namespace of the PSR interfaces that they tie back to, so these classes in the util package would keep their current namespace.

CRB
about.me/ashnazg

You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to php-fig+u...@googlegroups.com.

To post to this group, send email to php...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/CADqTB_hTs_vzN8raRbcCW0e3S31w%3DGHzno2rixzpzBQUim2-Nw%40mail.gmail.com.

[Sara Golemon]

On Friday, December 7, 2018 at 10:19:46 AM UTC-6, Rasmus Schultz wrote:

How is this in any way acceptable?

I'm strongly in favor of tests being in the same package as the thing being testing, and CI runs against that being part of the commit process, so in THAT one way I'd call it "acceptable".
However, I absolutely see your point about tying it to a specific testing framework, even if that framework is the de facto standard for composer based PHP projects.

Would you be less bothered by this if it were driven by PHP's own run-tests.php ?  That would give us reliable testing without any framework-favoring dependencies (depending on PHP is an avoidable given, after all).

 

I'm calling for a bugfix release to remove these ASAP.

I disagree with the urgency. Yes it's bad, and yes we should address fixing it, but nothing is broken *right now*, so we don't need to rush into it.

-Sara

[Woody Gilk]

As a user, I have no problem with a provided base test class using PHPUnit. Using PHP's run-tests.php would be a step backwards in my opinion.

However, I do agree that putting this into the package itself was a mistake, as was the version bump. It should be a -util package, as others have mentioned.

--
Woody Gilk
https://shadowhand.me

--

You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to php-fig+u...@googlegroups.com.
To post to this group, send email to php...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/a81c3c57-d084-4e47-a7ba-799af33d0ee5%40googlegroups.com.

[Rasmus Schultz]

> I'm strongly in favor of tests being in the same package as the thing being testing

Me too - but that's not what this is.

The package consists of interfaces - so there's nothing really to test, and there's no CI or anything like that set up. (and it wouldn't work if there were, because there's no dependency on PHPUnit although it's extending that class.)

This test is for implementations - so these are literally tests for other packages.

So yes, I'm entirely in favor of these tests residing in those packages - or a utility package, if the tests happen to be useful to other packages. (I'd go as far as naming that package with "phpunit" in the name, since these facilities aren't generic utilities - they're specific to phpunit, and the base-class depends on it.)

> nothing is broken *right now*, so we don't need to rush into it

I disagree - the damage is accumulating as people start to build things that depend on these facilities.

At the very least, an immediate bugfix release with deprecations should be released, so that people get informed of the mistake before the problem gets worse.

A mistake was made, and it's important to at least inform of that fact as quickly as possible - whether we do or don't actually remove it down the line at a later time... well, I'm not fond of things that light up red in my IDE because of undefined classes, but lets at least deprecate so people know it was a mistake?

--
You received this message because you are subscribed to a topic in the Google Groups "PHP Framework Interoperability Group" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/php-fig/DsmK600GAas/unsubscribe.
To unsubscribe from this group and all its topics, send an email to php-fig+u...@googlegroups.com.

[Hari K T]

Hi all,

 

The package consists of interfaces - so there's nothing really to test, and there's no CI or anything like that set up. (and it wouldn't work if there were, because there's no dependency on PHPUnit although it's extending that class.)

This test is for implementations - so these are literally tests for other packages.

So yes, I'm entirely in favor of these tests residing in those packages - or a utility package, if the tests happen to be useful to other packages. (I'd go as far as naming that package with "phpunit" in the name, since these facilities aren't generic utilities - they're specific to phpunit, and the base-class depends on it.)

> nothing is broken *right now*, so we don't need to rush into it

I disagree - the damage is accumulating as people start to build things that depend on these facilities.

At the very least, an immediate bugfix release with deprecations should be released, so that people get informed of the mistake before the problem gets worse.

A mistake was made, and it's important to at least inform of that fact as quickly as possible - whether we do or don't actually remove it down the line at a later time... well, I'm not fond of things that light up red in my IDE because of undefined classes, but lets at least deprecate so people know it was a mistake?

I have been going through this thread and I want to agree with Rasmus here. 

As php-fig is making interfaces there is no need for test packages. And if needed this can be a util package. Better late than never ? Because things will get worse with more time being given.

[Matthew Weier O'Phinney]

I totally disagree that there is no need for test packages.

The specifications for the interfaces are generally quite strict, but, particularly with versions that do not use PHP 7 (for scalar and return type hints), PHP 7.1 (nullable and void types), or PHP 7.2 (object type hint), having a test suite that can verify that the implementation conforms to the specification is quite useful, particularly for consumers who want a guarantee that swapping one implementation for another will result in the same behavior. We've had a number of issues with PSR-7 over this, and the test suite that Tobias Nyholm created helps implementors tremendously in this regard.

That said, I tend to agree these should be _separate packages_. I disagree with the urgency, but do agree it should be taken care of sooner rather than later.

--

Matthew Weier O'Phinney
mweiero...@gmail.com
https://mwop.net/

[Hari K T]

I totally disagree that there is no need for test packages.

Question here is which unit test framework ( may be phpunit ) ? What about others. Or all in one util package.

The specifications for the interfaces are generally quite strict, but, particularly with versions that do not use PHP 7 (for scalar and return type hints), PHP 7.1 (nullable and void types), or PHP 7.2 (object type hint), having a test suite that can verify that the implementation conforms to the specification is quite useful, particularly for consumers who want a guarantee that swapping one implementation for another will result in the same behavior. We've had a number of issues with PSR-7 over this, and the test suite that Tobias Nyholm created helps implementors tremendously in this regard.

That said, I tend to agree these should be _separate packages_. I disagree with the urgency, but do agree it should be taken care of sooner rather than later.

--

Matthew Weier O'Phinney
mweiero...@gmail.com
https://mwop.net/

--

You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.

To unsubscribe from this group and stop receiving emails from it, send an email to php-fig+u...@googlegroups.com.

To post to this group, send email to php...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/CAJp_myXiXQCXvYccMyV96oNJO0B_sDChiHnx-Aar%2Be71QSD7rQ%40mail.gmail.com.

[Larry Garfield]

Util packages have, I think, been using Fig\ as the top level namespace
segment, not Psr\. At least they're supposed to be... :-)

I am not too bothered if we use the *-util package or start a *-tests package
for this sort of case. But either way, they're not part of the spec and so
should never have been in the repo.

I'm squarely in the "move 'em out, tag 1.2, call it a day" camp. I think any
BC concerns are over-exaggerated unless demonstrated otherwise, and even then
fixing it for anyone affected is a single `composer require` statement.

--Larry Garfield

On Tuesday, December 11, 2018 7:39:05 AM CST Chuck Burgess wrote:
> At first glance, I think I'm good with moving this out to a log-util
> package, with perhaps a bump to v1.2. Adjusting testcases has never really
> urged me into being concerned about versioning, at least when working in my
> packages. The BC extent of anyone actually depending on these for their
> own tests should just be a `require` section adjustment in their
> `composer.json`, right? Namespacing of our other util packages that I
> spot-checked look like they stick to the namespace of the PSR interfaces
> that they tie back to, so these classes in the util package would keep
> their current namespace.
> CRB

> *about.me/ashnazg <http://about.me/ashnazg>*

[Robbie Averill]

Hi all,

>I'm squarely in the "move 'em out, tag 1.2, call it a day" camp.  I think any 

BC concerns are over-exaggerated unless demonstrated otherwise, and even then 
fixing it for anyone affected is a single `composer require` statement.

+1. I doubt anyone is depending on them, and the consensus seems to be that they shouldn't have been put there in the first place.

R

--
You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to php-fig+u...@googlegroups.com.
To post to this group, send email to php...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/9682261.mGdNde0gT2%40vulcan.

For more options, visit https://groups.google.com/d/optout.

--

Robbie Averill | Senior Developer

04 978 7330

http://silverstripe.com/