laravel / browser-kit-testing

Provides backwards compatibility for BrowserKit testing in the latest Laravel release.
MIT License
509 stars 75 forks source link

[6.x] Utilise illuminate/testing based on #122 #123

Closed crynobone closed 4 years ago

crynobone commented 4 years ago

There no breaking changes so far but we could clean-up some of the seeJson*** assertions which is available from TestResponse.

Signed-off-by: Mior Muhammad Zaki crynobone@gmail.com

crynobone commented 4 years ago

There might be some different usage based on this change.

Before

$response = $this->postJson('/uri', $data);

$this->assertResponseOk()->seeJson($json);

New structure

$response = $this->postJson('/uri', $data);

$response->assertResponseOk();

$this->seeJson($json);

I'm not sure whether we should move all the seeHelperAssertion to the new Laravel\ BrowserKitTesting\TestResponse or we should remove them since this is a major release.

crynobone commented 4 years ago

@driesvints welcome, btw we can do the same on Lumen Framework

taylorotwell commented 4 years ago

Can you update the changelog with the breaking changes and how to upgrade?

crynobone commented 4 years ago

As I explain we have few options that we can go with:

Changelog 1

All HTTP tests will now return an instance of Laravel\BrowserKitTesting\TestResponse instead of Illuminate\Http\Response. The advantage of having TestResponse is that you have access to every assertion available in Laravel HTTP Tests > Response Assertions.

Do that note that existing assertResponseOk(), assertResponseStatus(), assertViewHas(), assertViewHasAll(), assertViewMissing(), assertRedirectedTo(), assertRedirectedToRoute(), assertRedirectedToAction() and dump() need to be executed from TestResponse instance and no longer available from Laravel\BrowserKitTesting\TestCase context such as:

$response = $this->postJson('/uri', $data);

$reponse->assertResponseOk();

$this->seeJson($json);

or

$this->postJson('/uri', $data)
    ->assertResponseOk();

$this->seeJson($json);

This as of the current state of this PR.

Changelog 2

All HTTP tests will now return an instance of Laravel\BrowserKitTesting\TestResponse instead of Illuminate\Http\Response. The advantage of having TestResponse is that you have access to every assertion available in Laravel HTTP Tests > Response Assertions.

Do that note that existing shouldReturnJson(), receiveJson(), seeJsonEquals(), seeJson(), dontSeeJson(), seeJsonStructure(), seeJsonContains(), seeStatusCode(), seeHeader(), seePlainCookie(), seeCookie(), assertResponseOk(), assertResponseStatus(), assertViewHas(), assertViewHasAll(), assertViewMissing(), assertRedirectedTo(), assertRedirectedToRoute(), assertRedirectedToAction() and dump() need to be executed from TestResponse instance and no longer available from Laravel\BrowserKitTesting\TestCase context such as:

$response = $this->postJson('/uri', $data);

$reponse->assertResponseOk()->seeJson($json);

or

$this->postJson('/uri', $data)
    ->assertResponseOk()
    ->seeJson($json);

For Changelog 2, I need to move shouldReturnJson(), receiveJson(), seeJsonEquals(), seeJson(), dontSeeJson(), seeJsonStructure(), seeJsonContains(), seeStatusCode(), seeHeader(), seePlainCookie(), seeCookie() to Laravel\BrowserKitTesting\TestResponse.

crynobone commented 4 years ago

Personally, I think it is logical to accept Changelog 1 style due to the fact that might use it based on following:


$this->visit("users")->seeStatusCode(200)->seeCookie(...);

// the downside is you need explicitly request `$this->response` 
// to switch to assert the response.

$this->response->assertViewMissing(...); 
GrahamCampbell commented 4 years ago

What happened here?

crynobone commented 4 years ago

Resubmit a new PR without any breaking change