8000 [FrameworkBundle] Add `HttpClientAssertionsTrait` which provide shortcuts to assert HTTP calls was triggered by welcoMattic · Pull Request #50662 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[FrameworkBundle] Add HttpClientAssertionsTrait which provide shortcuts to assert HTTP calls was triggered #50662

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 2, 2023
Merged

[FrameworkBundle] Add HttpClientAssertionsTrait which provide shortcuts to assert HTTP calls was triggered #50662

merged 1 commit into from
Oct 2, 2023

Conversation

welcoMattic
Copy link
Member
@welcoMattic welcoMattic commented Jun 14, 2023 •
edited
Loading
Q A
Branch? 6.4
Bug fix? no
New feature? yes
Deprecations? no
Tickets
License MIT
Doc PR symfony/symfony-docs#18970

This PR brings a new collection (to enhance) of assertions to help users to ensure some HTTP calls has been triggered during a Request processing.

Example

An example is better than thousands words:

HttpClientAssertionsTest.php

public function testHttpClientAssertions()
{
    $client = static::createClient();
    $client->enableProfiler();

    $client->request('GET', '/foo');

    $this->assertHttpClientRequest($client, 'symfony.http_client', 'https://symfony.com/');
    $this->assertHttpClientRequest($client, 'symfony.http_client', 'https://symfony.com/', 'POST', 'foo');
    $this->assertHttpClientRequest($client, 'symfony.http_client', 'https://symfony.com/', 'POST', ['foo' => 'bar']);
    $this->assertHttpClientRequest($client, 'symfony.http_client', 'https://symfony.com/', 'POST', ['foo' => 'bar']);
    $this->assertNotHttpClientRequest($client, 'symfony.http_client', 'https://laravel.com');


    $this->assertHttpClientRequestCount($client, 'symfony.http_client', 5);
}

FooController.php

public function index(HttpClientInterface $symfonyHttpClient)
{
    $symfonyHttpClient->request('GET', '/');
    $symfonyHttpClient->request('POST', '/', ['body' => 'foo']);
    $symfonyHttpClient->request('POST', '/', ['body' => ['foo' => 'bar']]);
    $symfonyHttpClient->request('POST', '/', ['json' => ['foo' => 'bar']]);
    $symfonyHttpClient->request('GET', '/doc/current/index.html');

    return new Response();
}

http_client.yaml

framework:
    http_client:
        scoped_clients:
            symfony.http_client:
                base_uri: 'https://symfony.com'

This is a first try to improve DX in test, to allow users to assert more complex things (here it's HTTP calls, but I think also about adding assertions on logs for example).

Todo

  • Write doc

@carsonbot carsonbot added this to the 6.4 milestone Jun 14, 2023
@welcoMattic welcoMattic changed the title Add HttpClientAssertionsTrait which provide shortcut to assert HTTP calls was triggered [FrameworkBundle] Add HttpClientAssertionsTrait which provide shortcut to assert HTTP calls was triggered Jun 14, 2023
@welcoMattic welcoMattic force-pushed the profiler-asserts branch 2 times, most recently from b8c4786 to 92b819c Compare June 14, 2023 15:48
@welcoMattic welcoMattic force-pushed the profiler-asserts branch 3 times, most recently from 3a2eb65 to 1d5c59b Compare June 16, 2023 08:11
@welcoMattic
Copy link
Member Author

I've update the examples in PR description. Now we allow assertion on a single URL (with method and optional body). I've added the opposition assertion assertHttpClientHasNotBeenCalledForUrl and assertHttpClientCountRequests.

@OskarStark OskarStark changed the title [FrameworkBundle] Add HttpClientAssertionsTrait which provide shortcut to assert HTTP calls was triggered [FrameworkBundle] Add HttpClientAssertionsTrait which provide shortcut to assert HTTP calls was triggered Jun 19, 2023
nicolas-grekas
nicolas-grekas reviewed Aug 1, 2023
View reviewed changes
Copy link
Member
@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Random thoughts:

No need for checking headers using these?
Can't we remove the $client argument somehow? (don't we 8000 have it in the context somewhere?)
WHat about defaulting $httpClientId default to http_client?

@welcoMattic
Copy link
Member Author
welcoMattic commented Sep 19, 2023 •
edited
Loading

@nicolas-grekas

Random thoughts:

No need for checking headers using these?

Sure, I can add header checks 👍

Can't we remove the $client argument somehow? (don't we have it in the context somewhere?)

As BrowserKitAssertionTrait::getClient is private, we don't have any other way to get the client here. Should we dig deep to find a way to do?

We have access to this trait through WebTestAssertionsTrait in WebTestCase, so we can call static::getClient()

WHat about defaulting $httpClientId default to http_client?

Nice idea

@welcoMattic welcoMattic force-pushed the profiler-asserts branch 6 times, most recently from 28948b5 to 65e5d28 Compare September 19, 2023 13:54
@welcoMattic welcoMattic changed the title [FrameworkBundle] Add HttpClientAssertionsTrait which provide shortcut to assert HTTP calls was triggered [FrameworkBundle] Add HttpClientAssertionsTrait which provide shortcuts to assert HTTP calls was triggered Sep 19, 2023
@welcoMattic welcoMattic mentioned this pull request Sep 19, 2023
Closed
1 task
@weaverryan
Copy link
Member

Same thoughts as Nicolas: if we can remove the need for the client arg and service name (when using the default), I think I’ll really dig it!

@welcoMattic
Copy link
Member Author

@weaverryan it's done :) I've removed it https://github.com/symfony/symfony/pull/50662/files#diff-be0ff1fa989a87452ee0ecc8a932f09d50d59d3ecb4c6bce9565202aa254bd37R23.

Through traits we have access to getClient from BrowserKitAssertionsTrait, et tada!

fabpot
fabpot requested changes Oct 1, 2023
View reviewed changes
@welcoMattic welcoMattic mentioned this pull request Oct 2, 2023
Merged
@fabpot
Copy link
Member
fabpot commented Oct 2, 2023

Thank you @welcoMattic.

@fabpot fabpot merged commit 693276e into symfony:6.4 Oct 2, 2023
@welcoMattic welcoMattic deleted the profiler-asserts branch October 2, 2023 21:41
javiereguiluz added a commit to symfony/symfony-docs that referenced this pull request Oct 4, 2023
@javiereguiluz
minor #18970 [FrameworkBundle] Add HttpClientAssertions trait documen…
b331989 
…tation (welcoMattic)

This PR was merged into the 6.4 branch.

Discussion
----------

[FrameworkBundle] Add HttpClientAssertions trait documentation

Ref symfony/symfony#50662

Commits
-------

caa3483 Add HttpClientAssertions trait documentation
This was referenced Oct 21, 2023
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicolas-grekas nicolas-grekas nicolas-grekas left review comments

@stof stof stof left review comments

@OskarStark OskarStark OskarStark left review comments

@fabpot fabpot fabpot approved these changes

Assignees
No one assigned
Labels
Feature FrameworkBundle Status: Reviewed
Projects
None yet
Milestone
6.4
Development

Successfully merging this pull request may close these issues.
7 participants
@welcoMattic @weaverryan @fabpot @nicolas-grekas @stof @OskarStark @carsonbot
0