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

[8.x] Add assertExists testing method #38766

Merged
merged 2 commits into from
Sep 13, 2021
Merged

[8.x] Add assertExists testing method #38766

merged 2 commits into from
Sep 13, 2021

Conversation

RVxLab
Copy link
Contributor

@RVxLab RVxLab commented Sep 11, 2021

This PR adds the assertExists method for use in testing.

This method aims to make checking whether a model exists in the database easier, much like assertDeleted did for assertDatabaseMissing.

With this method, the following:

public function testProductIsNotDeleted(): void
{
    $product = Product::find(1);

    // ...

    $this->assertDatabaseHas('products', [
        'id' => $product->getKey(),
    ]);
}

Could be rewritten to:

public function testProductIsNotDeleted(): void
{
    $product = Product::find(1);

    // ...

    $this->assertExists($product);
}

@taylorotwell
Copy link
Member

Can you just assert on the exists property in your own tests?

$this->assertTrue($product->exists);

@RVxLab
Copy link
Contributor Author

RVxLab commented Sep 13, 2021

In a unit test that's definitely a possibility. While testing an API endpoint or a job where the model gets reloaded elsewhere that wouldn't work.

For instance a command that would prune inactive users, unless they have a subscription or whatever other criteria were set.

@bjhijmans
Copy link

bjhijmans commented Sep 13, 2021

I love it. I have this exact method in my app's testcase. I added it after being annoyed many times that there wasn't an assertion like it, and I was considering making this pr.

The only suggestion I have would be to call it assertNotDeleted. I like the symmetry of that name because it makes it immediately clear that it is the opposite assertion to assertDeleted. For assertExists it looks like its mirror should be assertDoesntExist, which doesn't exist. Exists could also refer to the $model->exists boolean that @taylorotwell mentioned, which may be confusing.

I may be missing use cases though. I only use it to test if something is deleted.

@RVxLab
Copy link
Contributor Author

RVxLab commented Sep 13, 2021

I love it. I have this exact method in my app's testcase. I added it after being annoyed many times that there wasn't an assertion like it, and I was considering making this pr.

The only suggestion I have would be to call it assertNotDeleted. I like the symmetry of that name because it makes it immediately clear that it is the opposite assertion to assertDeleted. For assertExists it looks like its mirror should be assertDoesntExist, which doesn't exist. Exists could also refer to the $model->exists boolean that @taylorotwell mentioned, which may be confusing.

I may be missing use cases though. I only use it to test if something is deleted.

You’re making a very good point here and I fully agree with you.

Perhaps assertNotDeleted is a much better idea. If this is desired I’ll make the changes.

As for use cases, I found myself using this when testing endpoints with deleting multiple records which don’t necessarily return a 422 or 403 when you’re not allowed to delete a thing.

This could also prove useful with pruning as stated in a previous comment of mine.

@taylorotwell taylorotwell merged commit 59ff96c into laravel:8.x Sep 13, 2021
@taylorotwell
Copy link
Member

Renamed to assertModelExists. Added assertModelMissing.

@caugner
Copy link
Contributor

caugner commented Sep 15, 2021

Renamed to assertModelExists. Added assertModelMissing.

@taylorotwell Could it make sense to introduce a Model assertion wrapper so that we can call $this->assertModel($model)->exists() and $this->assertModel($model)->missing() respectively?

PS: This would allow adding ->trashed() and similar.

victorvilella pushed a commit to cdsistemas/framework that referenced this pull request Oct 12, 2021
* [8.x] Add `assertExists` testing method

* add methods

Co-authored-by: Taylor Otwell <taylorotwell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants