Search code examples
laravellaravel-testing

Laravel Feature/Unit Testing


Since I am still rather a beginner in Laravel, I just stumbled upon an interesting issue with testing and need a second opinion.

I wrote models, controllers, views, factories, seeders and then (feature) tests. Tests do cover a lot (won't say all), but I just found out an issue which was not caught by tests, despite it should. Tests are mostly done by calling HTTP endpoints, so in theory it should show, add, edit and delete the data as user would. Factories are making sure that the entered data is about the same as user would enter (please let's leave out fraudulent users for this talk, that can change HTTP POST request).

So the issue was that model's attribute accepts true or false, factory only generates these two values for that attribute, but in controller when editing I wrongly set that if value is false or not present, it sets value to null. Strangely, despite I ran a lot of tests, this issue never showed up before, it only showed when I manually tried to update the model.

So, my questions:

  • why the tests didn't catch this issue, despite in some cases my edit call did send a false for that attribute?
  • how detailed are you writing the tests for your code? Are you testing ever possible outcome, with valid and invalid data, making a huge number of tests? Because testing all possible combinations will make number of tests huge.

Some code, since I know some of you will ask (note that I will only add relevant parts, not everything):

migration:

class CreateClientsTable extends Migration
{
    public function up()
    {
        Schema::create('clients', function (Blueprint $table) {
            $table->id();
            ...
            $table->boolean('invoice_details')->default(false);
            ...
        });
    }
    public function down()
    {
        Schema::dropIfExists('clients');
    }
}

model:

class Client extends Model
{
    use HasFactory;

    protected $fillable = [
        ...
        'invoice_details',
        ...
    ];

    ...

    protected static function newFactory(): Factory
    {
        return ClientFactory::new();
    }
}

controller:

class ClientController extends Controller
{
    ...
    
    public function edit(int $id): View
    {
        $data = Client::findOrFail($id);
        return view('client::edit', compact('data'));
    }

    public function update(UpdateClientRequest $request, int $id)
    {
        $client = Client::findOrFail($id);

        $response = $client->update([
            ...
            'invoice_details' => $request->invoice_details ?? null, // NOTE THE NULL, THIS TRIGGERED THE ERROR
            ...
        ]);

        ...
        
        return CustomResponse::resource($request, 'update', $response, route('client.index'), Client::findOrFail($id));
    }
}

factory:

class ClientFactory extends Factory
{
    protected $model = Client::class;

    public function definition(): array
    {
        $details = [
            ...
            'invoice_details' => $this->faker->randomElement([true, false]),
        ];

        if ($details['invoice_details']) {
            ...
        }

        return $details;
    }
}

seeder:

class ClientsSeeder extends Seeder
{
    public function run()
    {
        $client = Client::factory()->create();
    }
}

Error:

SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'invoice_details' cannot be null (SQL: update clients set invoice_details = ?, clients.updated_at = 2021-06-23 13:14:15 where id = 1)


Solution

  • I am writting with the cellphone, so have that in mind.

    The idea of tests is to test EVERYTHING, you wrote about not being sure if to write a lot of test or do it. The short answer is yes, do it, because that's the idea, test wvery possible outcome.

    Some endpoints are really 2 tests, some are 30... You did not catch this true/false (null) prpblem, because you did not test the endpoint getting a wrong value, null in this case or any that would create that value.

    So you have to send a wrong attribute (value) and expect an error back, when that happens, your test passes. Use $response->assertSessionHasError('invoice_details'). Look for it on the documentation as you can pass an array like ['attribute_name' => 'expected error message'].

    If you have more questions, update your question or create a new question with your code.