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

Xdebug support in PHPStorm #282

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Xdebug support in PHPStorm #282

wants to merge 14 commits into from

Conversation

haringsrob
Copy link

@haringsrob haringsrob commented Jun 7, 2017

Hi,

This is an initial proposal to add PHPstorm xdebug integration.

Current situation is that, you can debug behat as long as calls are direct, however, once http requests are involved like When I am on "\" then the debugger will no longer continue as no cookie is set.

This pull requests solves this by checking if the xdebug_config server variable is set, if that is the case, the session will receive the xdebug cookie allowing the debugger to work.

I have included internations/http-mock to mock a php webserver, we could update the other feature to use this as well. But let's handle that separate.

Travis is failing because internations/http-mock requires php7.. I'll see if I can test this in a different way.

Http mocking was not needed, still it is weird to use an online domain to do this.

Suggestions are welcome.

@haringsrob
Copy link
Author

Tests are failing because of external dependency's, I would drop php 5.3 support.

@@ -4,13 +4,13 @@ Feature: Search
I need to be able to search for a word

Scenario: Searching for a page that does exist
Given I am on "/wiki/Main_Page"
Given I am on "http://en.wikipedia.org/wiki/Main_Page"

Choose a reason for hiding this comment

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

@spolischook
Copy link

You should not use short array syntax to get compatible with php 5.3

@haringsrob
Copy link
Author

Hello @spolischook,

Changes are made.

As for your second comment, there is no short array syntax used anywhere in my commit.

composer.json Outdated
"behat/behat": "~3.0,>=3.0.5",
"behat/mink": "~1.5",
"symfony/config": "~2.2|~3.0"
"name": "behat/mink-extension",
Copy link
Member

Choose a reason for hiding this comment

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

please don't change the indentation of the file

.gitignore Outdated
@@ -2,3 +2,5 @@
*.phar
composer.lock
vendor
.idea
Copy link
Member

Choose a reason for hiding this comment

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

this one must be removed. It is specific to your system, not to the project, so it should not be added in the project-level ignore rules. See https://help.github.com/articles/ignoring-files/ to know how to ignore it on your system

/**
* @Then /^I should have the "([^"]*)" cookie with value "([^"]*)"$/
*/
public function iShouldHaveTheCookieWithValue($cookie_name, $cookie_expected_value) {
Copy link
Member

Choose a reason for hiding this comment

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

please respect our coding standards. We use PSR-2, so variable names are camelCased, and the curly brace should go on the next line (as done for the previous method)

if ($cookie_real_value !== $cookie_expected_value) {
throw new ExpectationException(
'The cookie with name ' . $cookie_name . ' was found, but does not contain ' . $cookie_real_value . ', yet it contains ' . $cookie_expected_value . '.',
$this->getSession()
Copy link
Member

Choose a reason for hiding this comment

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

passing the session in the constructor is deprecated. Please use the new API.

Btw, you could use the Mink assertion API for this: $this->assertSession()->cookieEquals(...) instead of duplicating its logic

@haringsrob
Copy link
Author

Hello @stof,

Thanks for the review, makes sense.

Changes have been applied.

@spolischook
Copy link

@haringsrob any way you need to fix tests before merge your impruvement https://travis-ci.org/Behat/MinkExtension/jobs/240659573#L350
Also, please remove hhvm from travis.yml, because it's not supported by travis-ci any more.
https://travis-ci.org/Behat/MinkExtension/jobs/240659577

@haringsrob
Copy link
Author

@spolischook I will remove the hhvm.

For the other part I don't agree,
PHP Parse error: syntax error, unexpected '[' in /home/travis/build/Behat/MinkExtension/vendor/behat/behat/src/Behat/Testwork/Argument/MixedArgumentOrganiser.php on line 288 As behat is no going to support php 5.3 anymore, why should we?

Also php 5.3 is already EOL for some time, http://php.net/eol.php, keeping on using it might give developer the wrong idea that it is ok.

@@ -7,11 +7,9 @@ cache:
- $HOME/.composer/cache/files

php:
- 5.3
Copy link
Member

Choose a reason for hiding this comment

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

As the min version in composer.json is still 5.3, this is invalid

Copy link
Author

Choose a reason for hiding this comment

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

Changed the composer.json now. Thanks, let me know if it is not the correct way.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants