Skip to content
This repository was archived by the owner on Apr 20, 2021. It is now read-only.

Reset request headers after send#164

Merged
sanpii merged 1 commit intoBehatch:masterfrom
vincentchalamon:master
Jul 24, 2017
Merged

Reset request headers after send#164
sanpii merged 1 commit intoBehatch:masterfrom
vincentchalamon:master

Conversation

@vincentchalamon
Copy link
Copy Markdown
Contributor

@vincentchalamon vincentchalamon commented Mar 26, 2016 •
edited
Loading

Q A
Bug fix? yes
New feature? no
BC breaks? yes
Deprecations? no
Tests pass? yes
Fixed tickets #163
License BEER-WARE

Requires FriendsOfPHP/Goutte#261

@vincentchalamon
Copy link
Copy Markdown
Contributor Author

vincentchalamon commented Apr 1, 2016

Ping @sanpii

@MLKiiwy
Copy link
Copy Markdown

MLKiiwy commented Apr 4, 2016

looks good 👍

@sanpii
Copy link
Copy Markdown
Member

sanpii commented Apr 4, 2016

Can you add a scenario to prevent regression?

@vincentchalamon
Copy link
Copy Markdown
Contributor Author

vincentchalamon commented Apr 4, 2016

@sanpii OK I'm on it. Do you have an idea how can I test with different Behat configurations? I would like to test with different clients (Goutte vs BrowserKit)

@sanpii
Copy link
Copy Markdown
Member

sanpii commented Apr 4, 2016

@vincentchalamon you can create a second behat configuration and use -c option to load it.

@vincentchalamon vincentchalamon mentioned this pull request Apr 4, 2016
Closed
@sanpii sanpii mentioned this pull request Apr 9, 2016
Closed
@ihorsamusenko
Copy link
Copy Markdown

ihorsamusenko commented Apr 9, 2016

Why can't we create new instance of Sanpi\Behatch\HttpCall\Request each time it is requested ?

@vincentchalamon
Copy link
Copy Markdown
Contributor Author

vincentchalamon commented Apr 11, 2016

@samusenkoiv You mean in RestContext?

@vincentchalamon
Copy link
Copy Markdown
Contributor Author

vincentchalamon commented Apr 11, 2016

@samusenkoiv Originally, Sanpi\Behatch\HttpCall\Request::getClient method recreates a request each time it's Sanpi\Behatch\HttpCall\Request receive any action (__call magic method). But if you request 2 methods on it (e.g. to add 2 headers), the first one will be lost as Request is recreated…

@webaaz
Copy link
Copy Markdown

webaaz commented Nov 10, 2016

Need this fix!

@vincentchalamon
Copy link
Copy Markdown
Contributor Author

vincentchalamon commented Nov 10, 2016

Still waiting for FriendsOfPHP/Goutte#261

fabpot added a commit to FriendsOfPHP/Goutte that referenced this pull request Nov 15, 2016
@fabpot
feature #261 Reset headers (vincentchalamon)
b47916e 
This PR was squashed before being merged into the 3.1-dev branch (closes #261).

Discussion
----------

Reset headers

Required by Behatch/contexts#164
See Behatch/contexts#163

Commits
-------

08724f1 Reset headers
@vincentchalamon
Copy link
Copy Markdown
Contributor Author

vincentchalamon commented Nov 15, 2016 •
edited
Loading

@sanpii @webaaz Current project requires PHP >=5.4, but fabpot/goutte requires PHP >=5.5. Can I upgrade this dependency in .travis.yml & composer.json files for production, or should I only force it for testing?

@webaaz
Copy link
Copy Markdown

webaaz commented Nov 16, 2016

http://php.net/supported-versions.php

@webaaz
Copy link
Copy Markdown

webaaz commented Nov 16, 2016

There's a 3.2 tag for goutte

@vincentchalamon
Copy link
Copy Markdown
Contributor Author

vincentchalamon commented Nov 17, 2016 •
edited
Loading

Scrutinizer failed to analyze atoum/atoum and recommended me to exclude it from analysis. @sanpii @webaaz Maybe is there a better way?

capture d ecran 2016-11-17 a 13 26 34

capture d ecran 2016-11-17 a 13 26 44

@elkangaroo
Copy link
Copy Markdown

elkangaroo commented Feb 28, 2017

Is there still anything preventing this fix from getting merged? This would also make PR #157 obsolete (which we currently use in our fork).

@sanpii sanpii added this to the 3.x milestone Mar 2, 2017
@elkangaroo
Copy link
Copy Markdown

elkangaroo commented Mar 31, 2017 •
edited
Loading

@vincentchalamon would you mind rebasing your branch against master and solve the conflict again? It would be really great to get this PR merged :)

@vincentchalamon
Copy link
Copy Markdown
Contributor Author

vincentchalamon commented Apr 3, 2017

@elkangaroo Fixed

@elkangaroo
Copy link
Copy Markdown

elkangaroo commented Apr 10, 2017

@sanpii I see that the only failing test has been failing occasionally on other builds too (and should probably be adjusted, because it makes the build unstable):

001 Scenario: Wait before seeing                                # tests/features/browser.feature:63
      Then the total elapsed time should be less than 6 seconds # tests/features/browser.feature:69

But other than that, do you think this PR can be merged?

@vincentchalamon
Copy link
Copy Markdown
Contributor Author

vincentchalamon commented Apr 11, 2017

@elkangaroo I'll try to update this PR today to remove deprecated warnings: https://travis-ci.org/Behatch/contexts/jobs/217980052

@vincentchalamon
Copy link
Copy Markdown
Contributor Author

vincentchalamon commented Apr 11, 2017 •
edited
Loading

@elkangaroo @sanpii Deprecated warnings fixed on Travis, I think it was due to forgotten namespaces on 86d0b77.
Please review to check I didn't break something bad ^^'

@elkangaroo
Copy link
Copy Markdown

elkangaroo commented Apr 11, 2017

Nice :)
But this 6 seconds test really sucks...
First it failed for PHP 5.5, now 5.5 is green and 5.6 - 7.1 fail. LOL

@sanpii
Copy link
Copy Markdown
Member

sanpii commented Apr 13, 2017 •
edited
Loading

But this 6 seconds test really sucks...

It sucks less now: c9ec184

@vincentchalamon
Copy link
Copy Markdown
Contributor Author

vincentchalamon commented Apr 28, 2017

Ping @sanpii

@sanpii sanpii self-requested a review April 28, 2017 12:31
@razbakov
Copy link
Copy Markdown
Contributor

razbakov commented May 9, 2017

Any update here? FriendsOfPHP/Goutte#261 is merged.

@razbakov razbakov mentioned this pull request May 9, 2017
Closed
razbakov
razbakov reviewed May 9, 2017
View reviewed changes
Comment thread src/HttpCall/Request/Goutte.php
@sanpii sanpii merged commit 4cc6cfc into Behatch:master Jul 24, 2017
@razbakov
Copy link
Copy Markdown
Contributor

razbakov commented Aug 21, 2017 •
edited
Loading

@vincentchalamon @sanpii can you please add a new release tag?

@gnutix gnutix mentioned this pull request Apr 15, 2021
Open
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@razbakov razbakov razbakov left review comments

@sanpii sanpii sanpii approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

3.0

Development

Successfully merging this pull request may close these issues.

7 participants

@vincentchalamon @MLKiiwy @sanpii @ihorsamusenko @webaaz @elkangaroo @razbakov