nothingworksinc / ticketbeast

Back to the lesson videos:
https://course.testdrivenlaravel.com/lessons
552 stars 11 forks source link

Writing authorisation tests for more than 3 roles #70

Open sileence opened 7 years ago

sileence commented 7 years ago

I can see why it is important to write different tests that hit the same URL with different kind of users (guests, promoters, admins, etc.)

But what should we do when working in a project that includes more than 5 roles. For example: guests, subscribers, collaborators, editors, admins, superadmins.

Should we write 1 test for each role in each URL?

I've been thinking quite a lot about this problem and I have some ideas:

hulkur commented 7 years ago

In situations where there are common behaviours on multiple different conditions I have used loops.

In your case:

for [guest, user, editor] do { login, do something, get access denied } and separate test for admin

vpratfr commented 7 years ago

Parametrized tests are the way to go for such cases. See @provider annotation from phpunit for more info.

sileence commented 7 years ago

Those looks like neat solutions, thank you both: @hulkur & @vpratfr

I was thinking about mocking the authorisation method and just checking it was called while testing the action and then unit testing the authorisation method in a separate unit test. I like that solution because I imagine it'd be faster but not having the integration/application test makes me feel uneasy.

MichaelDeBoey commented 7 years ago

@hulkur & @vpratfr That would mean that you have 1 huge test, with a different if-statement for each possible role I guess, so I think it would be much clearer if you just have separate tests isn't it?

sileence commented 7 years ago

There will be no conditionals and the test will be small, but it will accept parameters and be executed repeatedly:

    /**
     * @test
     * @dataProvider badBadRoles
     */
    function bad_roles_cannot_add_news($badRole)
    {
        $this->actingWithRole($badRole)
           ->post('news/create')
           ->assertAuthorizationError();
    }

    public function badBadRoles()
    {
        return [
            ['subscriber'],
            ['collaborator']
        ];
    }

Something like that if I understood the suggestion :)

vpratfr commented 7 years ago

Exactly. Where badBadRoles could be named provideUnauthorizedRoles for clarity ;)

sileence commented 7 years ago

I'm also thinking that with a little bit of ingenuity, it'd be easy to provide a list of all the actions in the app and the kind of users who have access to them :D

Nestecha commented 7 years ago

I think having the problem of too many tests means a problem at the root of the design of the code.

For example if you have a guest, a subscribed user and an admin and the two first do the same thing on some endpoints, you shouldn't be testing 2 different things because there should be something that have them in common and that very thing should be tested.

Say there's a free feature that anyone can use but only admins can edit it, then the concept of guest or subscribed user doesn't fit anymore, they're only regular users. So in the test you'd act as a regular user to test the endpoint, and that'd test both lower layer components.

TLDR : use a higher layer abstraction for stuff that multiple classes have in common :)

Le 23 oct. 2017 à 18:03, Duilio Palacios notifications@github.com<mailto:notifications@github.com> a écrit :

I'm also thinking that with a little bit of ingenuity, it'd be easy to provide a list of all the actions in the app and the kind of users who have access to them :D

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/nothingworksinc/ticketbeast/issues/70#issuecomment-338708850, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AHUtzFH-B0ms8C24LJZWI7JqMgFs1pD_ks5svLjPgaJpZM4QCreh.