move acting as trait to pests 'uses' function.

[myleshyson]

I noticed today that the teardown method on the acting as trait wasn't being run between tests. It probably has something to do with extending the base TestCase class. If the trait is moved into Pest.php it still works and allows projects to extend the base test case a class of their own.

uses(
    TestCase::class, // <--- my custom testcase class that extends markhuot\craftpest\test\TestCase
    InitializesDatabase::class, // <-- another custom trait with setup/teardown
    markhuot\craftpest\test\RefreshesDatabase::class,
    ActingAs::class, // <--- available for when the base test case calls "callTraits"
)->in('./');

[markhuot]

I’m trying to keep the default implementation of uses() of this as short as possible so as not to overwhelm new users. I’m hesitant to move the trait unless there’s a big benefit.

mid I can fix the tear down would you still need to override it?

[myleshyson]

The issue is more if anybody extends the TestCase class, then ActingAs just doesn't work, because callTraits is referencing my class now when searching for traits.

I think people should be able to extend the TestCase class and still have all the features working. Outside of that it also gives devs a chance to remove whatever they don't need, as maybe for some test suites you don't actually want to tear down a user session for whatever reason. I think methods are fine to keep in the base TestCase class. However anything relying on that callTraits method would really be moved Pest.php

Here's a concrete example of the issue.

// Pest.php
uses(
    TestCase::class, // <--- my test class
    markhuot\craftpest\test\RefreshesDatabase::class,
)->in('./');

// TestCase.php
class TestCase extends \markhuot\craftpest\test\TestCase
{
// ...project specifc stuff
}

// Example.php
test('some test creating a user and acting as them', function() {
    $user = User::factory()->admin(true)->create();
    $this->actingAs($user);
});

test('that previous user should be logged out for this test', function() {
    $user = Craft::$app->getUser()->getIdentity();
    expect($user)
        ->toBeNull();
});

And here's the results

[myleshyson]

Actually what do you think about seperating out the teardown logic from ActingAs and putting that in it's own trait, like RefreshUserSession? I looked at the laravel testing docs and it doesn't seem like they logout a user between each test in their actingAs method. They don't seem to provide a trait to handle that either so having one, however small it is, could be really helpful.

I still think any trait with a teardown/setup that affects how tests run should be offered in Pest.php, but I agree that any assertions should just come standard with the plugin. If it's split out then it's a win-win.

[markhuot]

I’m not sure here. Not tearing down the user, by default seems like a bug. That means reordering my tests could have varying results, which seems bad.

I have a PR to fix the bug, not accounting for subclassing the test case.

https://github.com/markhuot/craft-pest/pull/45

As for offering customization, I’d like to but I’m not ready yet. The plugin is still changing too much, it’s early days. I’m not ready to change Pest.php yet because each subsequent change makes upgrades more complex.

[myleshyson]

Yea that's fair. I'll go ahead and close this then.

[markhuot]

Merged in! I'm going to bookmark this in my notes though to come back to in a few versions. I like the idea of making all these traits customizable as soon as they settle a bit and I'm not changing them as frequently.