Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove assertArraySubset() #3495

Closed
sebastianbergmann opened this issue Jan 22, 2019 · 8 comments
Closed

Remove assertArraySubset() #3495

sebastianbergmann opened this issue Jan 22, 2019 · 8 comments
Assignees
Labels
type/backward-compatibility Something will be/is intentionally broken
Milestone

Comments

@sebastianbergmann
Copy link
Owner

The assertArraySubset() method is a constant source of confusion and frustration. For example, see #2069, #2108, #2568, #3101, #3234, #3240, or #3319.

I have come to the conclusion that this mess cannot be fixed without breaking backward compatibility. In addition to that, I myself have yet to see a use case for this or how this would be used then. I have also not seen this used in the wild.

This is why I decided to deprecate assertArraySubset() in PHPUnit 8 and to remove it in PHPUnit 9. Anybody who thinks this functionality is useful is more than welcome to take the code, put it into a separate project, and package it as an extension for PHPUnit.

@sebastianbergmann sebastianbergmann added feature-removal type/backward-compatibility Something will be/is intentionally broken labels Jan 22, 2019
@sebastianbergmann sebastianbergmann added this to the PHPUnit 9.0 milestone Jan 22, 2019
@sebastianbergmann sebastianbergmann self-assigned this Jan 22, 2019
@MatthewSH
Copy link

MatthewSH commented Jan 27, 2019

I just started really focusing on unit testing and I was actually looking for this functionality. For me, it makes sense and is useful.

In this test, I'm testing an array of arrays (friends). I always know one element will exist, however it's failing for the following reason.

Test failure

The response will look like this (with surrounding elements of course):

[
  ....
  [
    'account_id' => '36159956',
    'name' => 'athatcher7',
    'player_id' => '13023655',
    'ret_msg' => null,
  ]
  ...
]

I think the function could be useful...but after reading some of those issues...maybe a rewrite of the function would be better?

@godbout
Copy link
Sponsor

godbout commented Feb 15, 2019

Same as @MatthewSH. I use the assertion, I find it quite straightforward to understand actually. It will be missed!

@idimopoulos
Copy link

Indeed this assertion has caused me the same headache as by looking at the name I could argue that something like [b, c] is a subset of [a, b, c]. However, the assertion takes into account the keys as well.
There can be a lot of confusion around it indeed. Still, maybe we could have some brainstorming with having it replaced instead with a couple of other methods which are clearer, and also allow a parameter to handle some of these cases just like we use the $strict parameter.

@JKingweb
Copy link

I use assertArraySubset extensively in my tests, mostly with associative arrays, but also with sorted indexed arrays. Its removal would cause me more than a few headaches.

@TomasVotruba
Copy link

TomasVotruba commented Mar 13, 2019

Seeing issues like #2069 I agree.

Should keys match or just value? What if user needs that? How can we know?

@TomasVotruba
Copy link

@JKingweb Could you help me to cover your case in upgrade path? So you won't have to deal with that manually: rectorphp/rector#1193

@JKingweb
Copy link

I can certainly try. Please feel free to contact me with details on what you'd need.

@TomasVotruba
Copy link

TomasVotruba commented Mar 13, 2019

@JKingweb I've merged first dumb implementation.
I've added "Test it" section to the Pull-request, so you know how to test it: rectorphp/rector#1193

Report any issues found to Rector issues. There will be many in the start, but we'll try to handle one by one :)

@stale stale bot added the stale label Jul 2, 2019
Repository owner deleted a comment from stale bot Jul 2, 2019
jrfnl added a commit to jrfnl/phpunit-documentation-english that referenced this issue Feb 20, 2020
Based on the changelog of PHPUnit 9.0, the method has been removed, but it was still in the documentation.

Refs:
* https://github.com/sebastianbergmann/phpunit/blob/9.0.1/ChangeLog-9.0.md
* sebastianbergmann/phpunit#3495
sebastianbergmann pushed a commit to sebastianbergmann/phpunit-documentation-english that referenced this issue Feb 20, 2020
Based on the changelog of PHPUnit 9.0, the method has been removed, but it was still in the documentation.

Refs:
* https://github.com/sebastianbergmann/phpunit/blob/9.0.1/ChangeLog-9.0.md
* sebastianbergmann/phpunit#3495
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/backward-compatibility Something will be/is intentionally broken
Projects
None yet
Development

No branches or pull requests

6 participants