Skip to content

[FrameworkBundle] KernelTestCase: deprecate not using KERNEL_CLASS #22675

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

Merged
merged 1 commit into from
Jun 9, 2017
Merged

[FrameworkBundle] KernelTestCase: deprecate not using KERNEL_CLASS #22675

merged 1 commit into from
Jun 9, 2017

Conversation

ogizanagi
Copy link
Contributor

@ogizanagi ogizanagi commented May 9, 2017

Q A
Branch? 3.4
Bug fix? no
New feature? no
BC breaks? no
Deprecations? yes
Tests pass? yes
Fixed tickets #22668 (comment)
License MIT
Doc PR N/A

@@ -55,7 +55,7 @@ protected static function getPhpUnitXmlDir()

// Can't continue
if (null === $dir) {
throw new \RuntimeException('Unable to guess the Kernel directory.');
throw new \RuntimeException('Unable to guess the PHPUnit xml directory.');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess this method could be kept for BC and might somehow be useful. So I just update the exceptions messages rather than deprecating it too (same for getPhpUnitCliConfigArgument).
But what would be a proper exception message for:

if (!isset($_SERVER['argv']) || false === strpos($_SERVER['argv'][0], 'phpunit')) {
    throw new \RuntimeException('You must override the KernelTestCase::createKernel() method.');
}

some lines above (L.45) ?

Copy link
Member

Choose a reason for hiding this comment

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

XML instead of xml

UPGRADE-3.4.md Outdated

* Using `KERNEL_DIR` or the automatic guessing based on the `phpunit.xml` file
location is deprecated since 3.4. Either set `KERNEL_CLASS` in your `phpunit.xml`
to he fully-qualified class name of your Kernel or override the `KernelTestCase::createKernel()`
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo: he => the.

3.4.0
-----

* Deprecated not using the new `KERNEL_CLASS` server var with `KernelTestCase`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Deprecated ``KERNEL_DIR`` ... ?

Copy link
Contributor Author

@ogizanagi ogizanagi May 9, 2017

Choose a reason for hiding this comment

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

Not only. What is deprecated is the whole current method implementation, i.e KERNEL_DIR and the auto-detection based on phpunit.xml dir location. <=> Not using KERNEL_CLASS (it's better explained in the UPGRADE-3.4.md file. I believe it should stay short here)

Copy link
Contributor

Choose a reason for hiding this comment

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

If you override KernelTestCase::createKernel(), you (probably) don't use KERNEL_CLASS but you don't get a deprecation notice, this should be mentioned here as well. As this would make the sentence longer, I think @ro0NL's suggestions makes even more sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair enough. 😃

UPGRADE-3.4.md Outdated
* Using `KERNEL_DIR` or the automatic guessing based on the `phpunit.xml` file
location is deprecated since 3.4. Either set `KERNEL_CLASS` in your `phpunit.xml`
to the fully-qualified class name of your Kernel or override the `KernelTestCase::createKernel()`
method. Not setting `KERNEL_CLASS` will throw an exception on 4.0.
Copy link
Member

Choose a reason for hiding this comment

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

can't we also overwrite getKernelClass() only ?

Copy link
Contributor Author

@ogizanagi ogizanagi May 9, 2017

Choose a reason for hiding this comment

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

Yes indeed. I mean not setting KERNEL_CLASS when using the KernelTestCase::createKernel() base implementation. Of course if you overwrite it your on your own (and it's already suggested).
I don't know how to rephrase it better if the current version is not enough. I'll take any other suggestion.

EDIT: Sorry I misred your comment.

Copy link
Contributor

Choose a reason for hiding this comment

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

What about:

Using `KERNEL_DIR` or the automatic guessing based on the `phpunit.xml` file
location is deprecated since 3.4. Set `KERNEL_CLASS` to the fully-qualified
class name of your Kernel instead. Not setting `KERNEL_CLASS` will throw an
exception on 4.0 unless you override `KernelTestCase::createKernel()` or
`KernelTestCase::getKernelClass()` method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll take this. Thank you @julienfalque

@ogizanagi
Copy link
Contributor Author

I changed my mind about KernelTestCase::getPhpUnitXmlDir() and KernelTestCase::getPhpUnitCliConfigArgument() and I deprecated it as well (to be removed in 4.0). It's unlikely someone is using it for another reason (if it is, it's probably fragile) and has nothing to do in KernelTestCase once the current implementation of KernelTestCase::getKernelClass() is removed.

UPGRADE-4.0.md Outdated
@@ -331,6 +331,15 @@ FrameworkBundle
* The `Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\ValidateWorkflowsPass` class
has been removed. Use the `Symfony\Component\Workflow\DependencyInjection\ValidateWorkflowsPass`
class instead.

* The usage of `KERNEL_DIR` and the automatic guessing based on the `phpunit.xml`
file location has been removed from `KernelTestCase::getKernelClass()` implementation.
Copy link
Contributor

Choose a reason for hiding this comment

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

have been?

@javiereguiluz javiereguiluz added this to the 3.4 milestone May 11, 2017
UPGRADE-3.4.md Outdated
FrameworkBundle
---------------

* Using `KERNEL_DIR` or the automatic guessing based on the `phpunit.xml` file
Copy link
Member

Choose a reason for hiding this comment

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

phpunit.xml/phpunit.xml.dist?

UPGRADE-3.4.md Outdated
exception on 4.0 unless you override `KernelTestCase::createKernel()` or
`KernelTestCase::getKernelClass()` method.

* Methods `KernelTestCase::getPhpUnitXmlDir()` and `KernelTestCase::getPhpUnitCliConfigArgument()`
Copy link
Member

Choose a reason for hiding this comment

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

The methods [...]

*/
private static function getPhpUnitCliConfigArgument()
{
@trigger_error(sprintf('Method %s() is deprecated since 3.4 and will be removed in 4.0.', __METHOD__), E_USER_DEPRECATED);
Copy link
Member

Choose a reason for hiding this comment

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

The %s() method [...]

@@ -112,6 +120,8 @@ protected static function getKernelClass()
}

return $class;
} else {
@trigger_error(sprintf('Using KERNEL_DIR or the automatic guessing based on the phpunit.xml file location is deprecated since 3.4. Set KERNEL_CLASS to the fully-qualified class name of your Kernel instead. Not setting KERNEL_CLASS will throw an exception on 4.0 unless you override %1$::createKernel() or %1$::getKernelClass() method.', static::class), E_USER_DEPRECATED);
Copy link
Member

Choose a reason for hiding this comment

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

Using the KERNEL_DIR environment variable [...]

Copy link
Member

Choose a reason for hiding this comment

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

Set the KERNEL_CLASS environment variable [...]

*/
protected static function getPhpUnitXmlDir()
{
@trigger_error(sprintf('Method %s() is deprecated since 3.4 and will be removed in 4.0.', __METHOD__), E_USER_DEPRECATED);
Copy link
Member

Choose a reason for hiding this comment

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

The %s() method [...]

3.4.0
-----

* Deprecated using `KERNEL_DIR` server var with `KernelTestCase::getKernelClass()`.
Copy link
Member

Choose a reason for hiding this comment

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

Deprecated using the KERNEL_DIR environment variable [...]

-----

* Deprecated using `KERNEL_DIR` server var with `KernelTestCase::getKernelClass()`.
* Deprecated `KernelTestCase::getPhpUnitXmlDir()` and `KernelTestCase::getPhpUnitCliConfigArgument()` methods.
Copy link
Member

Choose a reason for hiding this comment

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

Deprecated the [...]

UPGRADE-4.0.md Outdated
override `KernelTestCase::createKernel()` or `KernelTestCase::getKernelClass()`
method instead.

* Methods `KernelTestCase::getPhpUnitXmlDir()` and `KernelTestCase::getPhpUnitCliConfigArgument()`
Copy link
Member

Choose a reason for hiding this comment

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

The methods [...]

@ogizanagi
Copy link
Contributor Author

Thanks for all the suggested improvements @xabbuh!

@ogizanagi ogizanagi changed the base branch from master to 3.4 May 17, 2017 18:31
@fabpot
Copy link
Member

fabpot commented Jun 9, 2017

Thank you @ogizanagi.

@fabpot fabpot merged commit d102fc0 into symfony:3.4 Jun 9, 2017
fabpot added a commit that referenced this pull request Jun 9, 2017
…KERNEL_CLASS (ogizanagi)

This PR was merged into the 3.4 branch.

Discussion
----------

[FrameworkBundle] KernelTestCase: deprecate not using KERNEL_CLASS

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | #22668 (comment)
| License       | MIT
| Doc PR        | N/A

Commits
-------

d102fc0 [FrameworkBundle] KernelTestCase: deprecate not using KERNEL_CLASS
@ogizanagi ogizanagi deleted the deprec/3.4/kernel_test_case_kernel_dir branch June 9, 2017 06:10
fabpot added a commit that referenced this pull request Jun 14, 2017
…e (ogizanagi)

This PR was merged into the 4.0-dev branch.

Discussion
----------

[FrameworkBundle] Remove KernelTestCase deprecated code

| Q             | A
| ------------- | ---
| Branch?       | master <!-- see comment below -->
| Bug fix?      | no
| New feature?  | no <!-- don't forget updating src/**/CHANGELOG.md files -->
| BC breaks?    | yes
| Deprecations? | no <!-- don't forget updating UPGRADE-*.md files -->
| Tests pass?   | yes
| Fixed tickets | #22675
| License       | MIT
| Doc PR        | N/A

(failures unrelated)

Commits
-------

6d28c43 [FrameworkBundle] Remove KernelTestCase deprecated code
@fabpot fabpot mentioned this pull request Oct 18, 2017
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.

8 participants