Phpunit: Deprecate assertArraySubset()

[sebastianbergmann]

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.

[jflambert]

Ah, we use assertArraySubset quite a lot with the current phpunit 7.5.2. Thanks for the early heads up, but could you provide some recommended alternatives?

[sebastianbergmann]

For assertArraySubset() there is no suggested alternative.

[solofeed]

@sebastianbergmann maybe will be better to improve this method or refactor instead of remove?

I think this method really useful

if I can help somehow, tell me about it

[rdohms]

I find it really useful to assert that an array has a set of keys/values instead of asserting one by one.

for example:

    public function fromExceptionWithDebugShouldAppendDebugInformation(): void
    {
        $dto       = ExceptionDto::fromExceptionWithDebug(new RuntimeException('Test', 123));
        $tracePart = [['function' => __FUNCTION__, 'class' => __CLASS__, 'type' => '->']];

        self::assertSame('Test', $dto->message);
        self::assertSame(RuntimeException::class, $dto->exception);
        self::assertSame(123, $dto->code);
        self::assertSame(__FILE__, $dto->file);
        self::assertSame(__LINE__ - 7, $dto->line);
        self::assertArraySubset($tracePart, $dto->trace);
        self::assertNull($dto->errorCode);
        self::assertNull($dto->violations);
    }

[maks-rafalko]

We were using it a lot. Decided to copy this code to our codebase after upgrading to PHPUnit 8.0 since no alternative has been suggested.

[GrahamCampbell]

What is this replaced by?

[GrahamCampbell]

Laravel is using this too.

[rdohms]

As suggested by @sebastianbergmann I have moved this to a separate package and added a thin layer for ease of use. If you are in need: https://packagist.org/packages/dms/phpunit-arraysubset-asserts

Will explore what we can do to make it clearer and more flexible as well, any input is welcome.

[clicars]

We use that method in every single test!! we need it!!

We are going to use the @rdohms package, if not all our tests will be destroyed.

[derrabus]

Well, PHPUnit 9 hasn't even been released yet and PHPUnit 8 will still be around for a while. Your tests are going to be fine.

[srosato]

Removing it will break backwards compatibility, might as well take the hit and use that BC break to remove the confusion. I find this method really useful. It helps us clearly show intent on what the actual test scenario really cares about. Without that function I can see some developers reverting back to assertEquals() by copy pasting entire arrays of data that is not relevant to the test's scenario intention.

[rdohms]

@derrabus depends on your config, mine for example break on deprecated warning.

[derrabus]

@rdohms I understand. But as far as I can tell, deprecating functionality is a normal thing to do with a minor release according to SemVer. I understand that you don't want to ignore deprecations until it's too late, but your configuration sounds a bit too strict to me.

[kubawerlos]

@srosato

Removing it will break backwards compatibility

Thus label backward-compatibility-break ;)

[leocavalcante]

Siler is using it as well, just got the warnings upgrading to PHPUnit 8.
It's a bummer to see it being deprecated without alternative suggestions, seems unconcerned with whoever relied on the library to write their tests.
If it was there it the first place, it provided API and people found an use case for that.

P.S.: it should be good to update the docs then: https://phpunit.readthedocs.io/en/8.0/assertions.html#assertarraysubset Add a warning telling newcomers that this feature is deprecated.

foreach ($expectedSubset as $key => $value) {
    $this->assertArrayHasKey($key, $actualArray);
    $this->assertSame($value, $actualArray[$key]);
}

[ElisDN]

It is helpful for API feature tests:

public function testAuth(): void
{
   $response = $this->post('/oauth2/auth', [
       'grant_type' => 'password',
        ...
   ]);

   self::assertEquals(200, $response->getStatusCode());
   self::assertJson($content = $response->getBody()->getContents());

   $data = json_decode($content, true);

   // Check subset of all or needed static values:

   self::assertArraySubset([
       'token_type' => 'Bearer',
        ...
   ], $data);

   // Check other generated values:

   self::assertArrayHasKey('access_token', $data);
   self::assertNotEmpty($data['access_token']);
}

[rdarcy1]

+1 for API tests, very useful to be able to check part of a response is present without having to check the whole thing.

Often I'll have a test for the whole response, and then separate tests to check that certain attributes change based on different setups. In these cases there's no need to check the whole response array again because it's already covered.

[module17]

This one is used quite a bit. Debate the deprecation.

[deviouspk]

Please do not deprecate this without a proper alternative. It's used in many projects and even big frameworks (e.g. laravel)

[Namoshek]

Using this method myself in a few tests to ensure that payloads sent to and received from APIs contain the data required and also that the payloads do not contain any excess data we don't want to send.

Due to the fact that we also send data that is not persisted anywhere, we cannot perform simple string comparison of serialized JSON. Some data only lives during creation of the requests. So we need structural comparison as provided by assertArraySubset().

What does work as alternative for me is the following:

$expected = [
    'foo' => [
        'bar' => 123
    ],
    'baz' => 'hello world!'
];

$result = $someClass->myTestedMethod();

// phpunit v7
$this->assertArraySubset($expected, $result); // compare structure only
$this->assertArraySubset($expected, $result, true); // compare structure and values strictly

// phpunit v8
$this->assertTrue(
    empty(array_diff_key($expected, $result)) && empty(array_diff_key($result, $expected)
); // compare structure (keys) only
$this->assertTrue(
    empty(array_diff_assoc($expected, $result)) && empty(array_diff_assoc($result, $expected)
); // compare structure (keys) and values strictly

Actually, using array_diff_*($expected, $data) is more accurate. It will not allow for excess data in $result, which assertArraySubset() seems to do.

[MusikAnimal]

If it means anything, MediaWiki (the software that powers Wikipedia) also uses assertArraySubset(), as does Symfony, and a number of related libraries and extensions. Perhaps they don't have to, just making note of it.

[ahinkle]

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.

@sebastianbergmann This depreciation is surprising and somewhat frustrating as assertArraySubset is widely used across many frameworks and packages with no debate or alternatives.

[sebastianbergmann]

All I have to say on this subject I have written down here.

If you rely on assertArraySubset() and its current behaviour and want to continue to use it then, please, go ahead, take the code, and put it into an extension. Not every assertion needs to be part of PHPUnit's standard distribution.

[deviouspk]

@sebastianbergmann honestly packaging such a widely spread and basic assertion into a separate extension and thereby forcing people to rely on a less stable and maintained extension for again basic minor functionality, does not seem like a logical choice at all. Why not decide on the behavior of the method and add it as a new properly documented method? It seems like most of the frustrations are being caused when asserting a subset of a non associative array.

For most developers when they want to assert that a certain array is a subset of a non associative array. They consider the keys as something to identify the "order" of the elements in the array and not as something to identify the element by. So having one method to deal with both associative and non associative arrays didn't make sense in the first place anyway.

What would make sense is having methods like this:

assertAssocArraySubset($subset, $array, $strict=false);

assertIndexArraySubset($subset, $array, $order=false, $strict=false);

The behavior for the _assertAssocArraySubset_ method would be exactly the same as the current assertArraySubset method.

The behavior for the _assertIndexArraySubset_ method would differ from _assertAssocArraySubset_ in the way that it would not consider the keys of the elements. It will also introduce a boolean _order_ that would be false by default and checks if the elements in the subset have the same order as in the array. To illustrate this with an example i'll reference the issues encountered in #2069 .

1. $this->assertIndexArraySubset( ['a', 'b', 'c'], ['a', 'b', 'c', 'd'] );
2. $this->assertIndexArraySubset( ['b', 'c'], ['a', 'b', 'c', 'd'] );
3. $this->assertIndexArraySubset( ['a', 'c', 'b'], ['a', 'b', 'c', 'd'] );
4. $this->assertIndexArraySubset( ['a', 'c', 'b'], ['a', 'b', 'c', 'd'], true);
5. $this->assertIndexArraySubset( ['a', 'd,' 'c'], ['a', 'b', 'c', 'd'], true);
6. $this->assertIndexArraySubset( ['a', 'c', 'd'], ['a', 'b', 'c', 'd'], true);

Respectively this would return:

1. true
2. true
3. true
4. false
5. false
6. This would be the only debatable exception and the behavior for this case could be discussed. I would prefer to make it return false. Especially because you could have multiple elements with the same value. In my opinion the method should only check if the subset array is a subset and has the order of that part of the array and not look any further into the array then that.

Last but not least.
If this method is actually removed in phpunit 9 everyone will start implementing their own version of it (I doubt that there will be a stable specific extension that everyone will use to implement this functionality.). Causing a lot more confusion and frustration for people that are switching frameworks & codebases. Since behavior might differ from framework to framework. A big testing framework like phpunit has the responsability to set the standard and decide on the default behavior for basic assertions. Removing this would be a mistake.

I really hope we can give this another chance :)

Best regards,
Arthur

[sebastianbergmann]

The excellent @rdohms got you covered: https://packagist.org/packages/dms/phpunit-arraysubset-asserts