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

Tests: ResourceControllerTest store_saves() fails due faulty test method #207

Closed
axit-joost opened this issue May 15, 2020 · 12 comments
Closed
Labels
bug Something isn't working in progress This issue is being worked on.

Comments

@axit-joost
Copy link
Contributor

axit-joost commented May 15, 2020

  • Laravel Version: 7.11.0
  • PHP Version: 7.4.4
  • Blueprint Version: 1.11.0
  • Platform: Mac

Description:

Tests generated for a resource: api controller do not use a factory.

    public function store_saves()
    {
        $certificate = $this->faker->word; // shouldn't this whip up a new certificate?

        $response = $this->post(route('certificate.store'), [
            'certificate' => $certificate,
        ]);

        $certificates = Certificate::query()
            ->where('certificate', $certificate) // shouldn't this lookup $certificate->id (or ::find() it?
            ->get();
        $this->assertCount(1, $certificates);
        $certificate = $certificates->first(); // not sure why this is here?
    }

Also, looking at the other tests, I think that the Test Generator can do with a bit of tender love and care: A lot of tests do not assert anything.

Steps To Reproduce:

  1. Add the following contents to draft.yaml:
models:
  Certificate:
    name: string
    certificate_type_id: id
    reference: string
    document: string
    expiry_date: date
    remarks: nullable text
  CertificateType:
    name: string
    relationships:
      hasMany: Certificate

controllers:
  Certificate:
    resource: api
  CertificateType:
    resource: api
  1. Issue a php artisan blueprint:build, which will yield:
❯ a blueprint:build                                                                                                                                                     48%
Created:
- database/migrations/2020_05_15_072744_create_certificates_table.php
- database/migrations/2020_05_15_072745_create_certificate_types_table.php
- app/Certificate.php
- app/CertificateType.php
- database/factories/CertificateFactory.php
- database/factories/CertificateTypeFactory.php
- app/Http/Controllers/CertificateController.php
- app/Http/Controllers/CertificateTypeController.php
- app/Http/Requests/CertificateStoreRequest.php
- app/Http/Requests/CertificateUpdateRequest.php
- app/Http/Requests/CertificateTypeStoreRequest.php
- app/Http/Requests/CertificateTypeUpdateRequest.php
- app/Http/Resources/CertificateCollection.php
- app/Http/Resources/Certificate.php
- app/Http/Resources/CertificateTypeCollection.php
- app/Http/Resources/CertificateType.php
- tests/Feature/Http/Controllers/CertificateControllerTest.php
- tests/Feature/Http/Controllers/CertificateTypeControllerTest.php

Updated:
- routes/web.php
  1. Run phpunit, which yields:
❯ phpunit                                                                                                                                                               48%
PHPUnit 8.5.4 by Sebastian Bergmann and contributors.

..R.FR.R.R.FR.R.                                                  16 / 16 (100%)

Time: 457 ms, Memory: 30.00 MB

There were 2 failures:

1) Tests\Feature\Http\Controllers\CertificateControllerTest::store_saves
Failed asserting that actual size 0 matches expected size 1.

/Users/joostjacobs/Code/blueprint/tests/Feature/Http/Controllers/CertificateControllerTest.php:55

2) Tests\Feature\Http\Controllers\CertificateTypeControllerTest::store_saves
Failed asserting that actual size 0 matches expected size 1.

/Users/joostjacobs/Code/blueprint/tests/Feature/Http/Controllers/CertificateTypeControllerTest.php:55

--

There were 6 risky tests:

1) Tests\Feature\Http\Controllers\CertificateControllerTest::index_behaves_as_expected
This test did not perform any assertions

/Users/joostjacobs/Code/blueprint/tests/Feature/Http/Controllers/CertificateControllerTest.php:21

2) Tests\Feature\Http\Controllers\CertificateControllerTest::show_behaves_as_expected
This test did not perform any assertions

/Users/joostjacobs/Code/blueprint/tests/Feature/Http/Controllers/CertificateControllerTest.php:63

3) Tests\Feature\Http\Controllers\CertificateControllerTest::update_behaves_as_expected
This test did not perform any assertions

/Users/joostjacobs/Code/blueprint/tests/Feature/Http/Controllers/CertificateControllerTest.php:86

4) Tests\Feature\Http\Controllers\CertificateTypeControllerTest::index_behaves_as_expected
This test did not perform any assertions

/Users/joostjacobs/Code/blueprint/tests/Feature/Http/Controllers/CertificateTypeControllerTest.php:21

5) Tests\Feature\Http\Controllers\CertificateTypeControllerTest::show_behaves_as_expected
This test did not perform any assertions

/Users/joostjacobs/Code/blueprint/tests/Feature/Http/Controllers/CertificateTypeControllerTest.php:63

6) Tests\Feature\Http\Controllers\CertificateTypeControllerTest::update_behaves_as_expected
This test did not perform any assertions

/Users/joostjacobs/Code/blueprint/tests/Feature/Http/Controllers/CertificateTypeControllerTest.php:86

FAILURES!
Tests: 16, Assertions: 20, Failures: 2, Risky: 6.
  1. Inspect the CertificateControllerTest, particularly the store_saves test:
    /**
     * @test
     */
    public function store_saves()
    {
        $certificate = $this->faker->word;

        $response = $this->post(route('certificate.store'), [
            'certificate' => $certificate,
        ]);

        $certificates = Certificate::query()
            ->where('certificate', $certificate)
            ->get();
        $this->assertCount(1, $certificates);
        $certificate = $certificates->first();
    }
@axit-joost axit-joost added bug Something isn't working pending This issue is pending review labels May 15, 2020
@axit-joost
Copy link
Contributor Author

Proposal for the generated test:

    /**
     * @test
     */
    public function store_saves()
    {
        // Whip up a certificate using the factory
        $certificate = factory(Certificate::class)->make();

        // Post it to the appropriate route
        $response = $this->post(route('certificate.store'), $certificate->toArray());

        // Assert the 201 Created status
        $response->assertCreated();

        $certificates = Certificate::query()
            ->where('id', $response['data']['id']) // Lookup using the id from the returned Resource
            ->get();

        // Assert that there is only one record
        $this->assertCount(1, $certificates);
    }

That last bit of verifying the count is a bit double up if you ask me. assertCreated() asserts what the test wants to test: that store saves.

@jasonmccreary
Copy link
Collaborator

I don't think this is an issue in and of itself. However, it is a result of whatever is broken in #209.

@axit-joost
Copy link
Contributor Author

I got notified of the merge and subsequent release that fixed #209, but the generated tests remain broken.

@jasonmccreary
Copy link
Collaborator

Yes, that's why this is still open. 👍

@axit-joost
Copy link
Contributor Author

The fixtures in the test suite also contain the same broken code for the store_saves(). I would propose changing the contents of the fixture to be something that does work and then TDD'ing it to green.

@jasonmccreary
Copy link
Collaborator

@axit-joost, you seem to be pretty familiar with the codebase. Would you care to try and resolve some of these bugs you're reporting?

@axit-joost
Copy link
Contributor Author

@jasonmccreary Sure, I'd like to give it a go, but usually @Pr3d4dor beats me to it by squashing my bugs overnight 😅 (thanks a lot by the way!)

@jasonmccreary
Copy link
Collaborator

Yeah, you two and @nathane need to share your Twitter handles so I can give y'all a shout out. ❤️

@axit-joost
Copy link
Contributor Author

@joostjacobs on twitter.

@Pr3d4dor
Copy link
Contributor

My twitter handle is @GianlucaBine

@jasonmccreary jasonmccreary added in progress This issue is being worked on. and removed pending This issue is pending review labels May 20, 2020
@axit-joost
Copy link
Contributor Author

For those who are interested, I have packaged up the declined PR.

Blueprint allows for Generators to be swapped. My package will swap the shipped TestGenerator with a StreamlinedTestGenerator that will output a very opinionated, yet working test suite in the case where the draft.yaml utilizes the resource: api shorthand in the controllers: section.

GitHub repo here:
https://github.com/axitbv/laravel-blueprint-streamlined-test-addon

Install it via composer:
composer require --dev axitbv/laravel-blueprint-streamlined-test-addon

I will be using that until this issue is properly resolved.

Hat tip to @dmason30 for inspiring me with his Blueprint Pest Addon

@jasonmccreary
Copy link
Collaborator

The remaining generation of fake data using factories should be resolved by #282. I will wait to tag master until early next week.

I also opened #283 to focus on the aspect of risky tests being generated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working in progress This issue is being worked on.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants