Skip to content
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.

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

Check if Client exists when test.client does not exist, to provide clearer exception message #30479

Merged
merged 1 commit into from Mar 20, 2019
Merged

Check if Client exists when test.client does not exist, to provide clearer exception message #30479

merged 1 commit into from Mar 20, 2019

Conversation

SerkanYildiz
Copy link
Contributor

@SerkanYildiz SerkanYildiz commented Mar 7, 2019

Q A
Branch? 3.4
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #30420
License MIT

The DotEnv component does not overwrite by using environment variables declared in .env files.

In the FrameworkExtension will be checked if the framework.test config is set to a non-false value, if so it will load the test.xml file which contains the definition for the test.client service.

When running php bin/phpunit it will use phpunit.xml.dist but because we defined APP_ENV in our system it will not load test.xml so when creating a client to do functional tests, we'll get an exception which isn't correct: You cannot create the client used in functional tests if the BrowserKit component is not available. Try running "composer require symfony/browser-kit"

This PR aims to add a clearer exception message which indicates what really should be done to fix the error message.

@SerkanYildiz SerkanYildiz changed the title Check if BrowserKit/Client Check if Client exists when test.client does not exist, to provide clearer exception message Mar 7, 2019
@SerkanYildiz SerkanYildiz changed the base branch from master to 3.4 March 7, 2019 15:43
@nicolas-grekas nicolas-grekas added this to the 3.4 milestone Mar 9, 2019
@SerkanYildiz
Copy link
Contributor Author

ping @nicolas-grekas
can you re-review and if ok accept the PR?

@fabpot
Copy link
Member

fabpot commented Mar 20, 2019

Thank you @SerkanYildiz.

@fabpot fabpot merged commit b429950 into symfony:3.4 Mar 20, 2019
fabpot added a commit that referenced this pull request Mar 20, 2019
… provide clearer exception message (SerkanYildiz)

This PR was squashed before being merged into the 3.4 branch (closes #30479).

Discussion
----------

Check if Client exists when test.client does not exist, to provide clearer exception message

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #30420
| License       | MIT

The `DotEnv` component does not overwrite by using environment variables declared in `.env` files.

In the `FrameworkExtension` will be checked if the framework.test config is set to a non-false value, if so it will load the `test.xml` file which contains the definition for the `test.client` service.

When running `php bin/phpunit` it will use `phpunit.xml.dist` but because we defined `APP_ENV` in our system it will not load `test.xml` so when creating a client to do functional tests, we'll get an exception which isn't correct: `You cannot create the client used in functional tests if the BrowserKit component is not available. Try running "composer require symfony/browser-kit"`

This PR aims to add a clearer exception message which indicates what really should be done to fix the error message.

Commits
-------

b429950 Check if Client exists when test.client does not exist, to provide clearer exception message
@SerkanYildiz SerkanYildiz deleted the fix-logicexception-in-webtestcase branch March 20, 2019 07:53
fabpot added a commit that referenced this pull request Mar 23, 2019
…rkanYildiz)

This PR was merged into the 4.3-dev branch.

Discussion
----------

[FrameworkBundle] Update Client class to KernelBrowser

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| License       | MIT

I updated recently the WebTestCase with a check to provide a better/more clear exception message (see #30479). After that change @fabpot renamed different `Client` classes in components to a clearer name (for ex Client in HttpKernel is now KernelBrowser etc.). This PR aims to replace the Client in WebTestCase to the new name class name.

Commits
-------

28b6dd2 Replace class with new name.
This was referenced Apr 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants