pestphp / pest

Pest is an elegant PHP testing Framework with a focus on simplicity, meticulously designed to bring back the joy of testing in PHP.
https://pestphp.com
MIT License
9.49k stars 343 forks source link

[Bug]: Arch test not working properly with array values in 'expect' #1035

Closed me-shaon closed 8 months ago

me-shaon commented 9 months ago

What Happened

I wanted to write an Architectural test like the following:

arch('Do not access session data in jobs')
    ->expect(['session', 'auth'])
    ->not->toBeUsedIn('App\Jobs');

to make sure none of session or auth methods are used in Job classes. But the test is not working and returns success even if I use any of the above methods.

But if I use a single value instead of an array in the expect like the following:

arch('Do not access session data in jobs')
    ->expect('session')
    ->not->toBeUsedIn('App\Jobs');

then it's working fine.

How to Reproduce

Just add the test as mentioned above and use any of the methods in the class and see that the test is not failing.

Sample Repository

No response

Pest Version

2.28.0

PHP Version

8.1

Operation System

macOS

Notes

No response

owenvoke commented 9 months ago

Running this test, the expectation does fail, however it then seems to be ignored for some reason. 🤔

The docs don't actually mention that using an array is supported, so I'd go with the singular tests for now. However, this this does seem like it might be a bug. 🤷🏻

me-shaon commented 9 months ago

@owenvoke the doc has example for toBeUsed with an array. That's why it looks like toBeUsedIn should work with an array as well.

I've a use case in my mind that requires 5-6 elements for this test. So, putting separate test case for each of those would be cumbersome. 😢

owenvoke commented 9 months ago

Yeah, I totally agree that this is weird that it doesn't work, and that it'd be cumbersome and look poor having to do 5-6 tests. Definitely something to be investigated.

me-shaon commented 8 months ago

@nunomaduro can you please take a look? I don't know how to fix this, otherwise, I'd send a PR.

byOskar commented 8 months ago

If you append each after the expectation array, this should work. https://pestphp.com/docs/expectations#content-each

arch('Do not access session data in jobs')
    ->expect(['session', 'auth'])
+   ->each->not->toBeUsedIn('App\Jobs');
-   ->not->toBeUsedIn('App\Jobs');
devajmeireles commented 8 months ago

If you append each after the expectation array, this should work. https://pestphp.com/docs/expectations#content-each

arch('Do not access session data in jobs')
    ->expect(['session', 'auth'])
+   ->each->not->toBeUsedIn('App\Jobs');
-   ->not->toBeUsedIn('App\Jobs');

This is the correct approach of usage when you're trying to check multiple functions in expect.

Since we have a correct usage model, I'll close this issue. Feel free to open a new issue whenever necessary.