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

Reset request headers after send #164

Merged
merged 1 commit into from
Jul 24, 2017
Merged

Reset request headers after send #164

merged 1 commit into from
Jul 24, 2017

Conversation

vincentchalamon
Copy link
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
Contributor Author

Ping @sanpii

@MLKiiwy
Copy link

MLKiiwy commented Apr 4, 2016

looks good 👍

@sanpii
Copy link
Member

sanpii commented Apr 4, 2016

Can you add a scenario to prevent regression?

@vincentchalamon
Copy link
Contributor Author

@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
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

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

@vincentchalamon
Copy link
Contributor Author

@samusenkoiv You mean in RestContext?

@vincentchalamon
Copy link
Contributor Author

@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

webaaz commented Nov 10, 2016

Need this fix!

@vincentchalamon
Copy link
Contributor Author

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
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

webaaz commented Nov 16, 2016

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

@webaaz
Copy link

webaaz commented Nov 16, 2016

There's a 3.2 tag for goutte

@vincentchalamon
Copy link
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

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

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
Contributor Author

@elkangaroo Fixed

@elkangaroo
Copy link

@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
Contributor Author

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

@vincentchalamon
Copy link
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

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
Member

sanpii commented Apr 13, 2017 •
edited
Loading

But this 6 seconds test really sucks...

It sucks less now: c9ec184

@vincentchalamon
Copy link
Contributor Author

Ping @sanpii

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

razbakov commented May 9, 2017

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

@razbakov razbakov mentioned this pull request May 9, 2017
Closed
@sanpii sanpii merged commit 4cc6cfc into Behatch:master Jul 24, 2017
@razbakov
Copy link
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

@razbakov razbakov razbakov left review comments

@sanpii sanpii sanpii approved these changes

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