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.

Sign up for GitHub

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

Jump to bottom

[FrameworkBundle] Rename internal WebTestCase to avoid confusion #32577

Closed
weaverryan opened this issue Jul 17, 2019 · 6 comments
Closed

[FrameworkBundle] Rename internal WebTestCase to avoid confusion #32577

weaverryan opened this issue Jul 17, 2019 · 6 comments
Labels
DX DX = Developer eXperience (anything that improves the experience of using Symfony) FrameworkBundle Good first issue Ideal for your first contribution! (some Symfony experience may be required)

Comments

@weaverryan
Copy link
Member

Description
FrameworkBundle has two classes called WebTestCase:

The second is only meant for our internal tests. But because it has the same-name, it's easy for a user to extend the wrong one from their own tests. There is also another internal WebTestCase inside SecurityBundle.

Has this been discussed before? It seems like an easy win to rename this internal testing class to something else.
@javiereguiluz javiereguiluz added DX DX = Developer eXperience (anything that improves the experience of using Symfony) FrameworkBundle Good first issue Ideal for your first contribution! (some Symfony experience may be required) labels Jul 17, 2019
@fabpot
Copy link
Member

fabpot commented Jul 17, 2019

As this is an internal class, renaming is not a BC-break. We should find a new name though. What about FunctionalWebTestCase?

@ro0NL
Copy link
Contributor

ro0NL commented Jul 17, 2019 •
edited

isnt any "web test case" is a functional concern already 🤔

symfony/src/Symfony/Bundle/FrameworkBundle/Test/WebTestCase.php

Line 18 in 25f1804

* WebTestCase is the base class for functional tests.

for the Functional namespace it could be a Abstract(Web)TestCase

@fabpot
Copy link
Member

fabpot commented Jul 18, 2019

AbstractWebTestCase works for me.

janvt added a commit to janvt/symfony that referenced this issue Jul 19, 2019
@janvt
Rename internal WebTestCase to AbstractWebTestCase
c3251a9 
FrameworkBundle had 2 classes called WebTestCase, one of which is only
meant for internal tests. To avoid confusion the internal class has
been renamed to AbstractWebTestCase

'See symfony#32577
@janvt janvt mentioned this issue Jul 19, 2019
Closed
@ismail1432 ismail1432 mentioned this issue Jul 19, 2019
Closed
@janvt
Copy link
Contributor

janvt commented Jul 19, 2019

The same is the case in the SecurityBundle - \Symfony\Bundle\SecurityBundle\Tests\Functional\WebTestCase. Currently working on a PR, should I rename this as well? Seems to be internal, same as the FrameworkBundle class.

@xabbuh
Copy link
Member

xabbuh commented Jul 19, 2019

Sounds reasonable to me.

janvt added a commit to janvt/symfony that referenced this issue Jul 19, 2019
@janvt
Rename internal WebTestCase to AbstractWebTestCase
0765272 
SecurityBundle had 2 classes called WebTestCase, one of which is only
meant for internal tests. To avoid confusion the internal class has
been renamed to AbstractWebTestCase and made abstract.

See symfony#32577
janvt added a commit to janvt/symfony that referenced this issue Jul 19, 2019
@janvt
[SecurityBundle] Rename internal WebTestCase to AbstractWebTestCase
00a8405 
SecurityBundle had 2 classes called WebTestCase, one of which is only
meant for internal tests. To avoid confusion the internal class has
been renamed to AbstractWebTestCase and made abstract.

See symfony#32577
janvt added a commit to janvt/symfony that referenced this issue Jul 19, 2019
@janvt
[SecurityBundle] Rename internal WebTestCase to AbstractWebTestCase
a779933 
SecurityBundle had 2 classes called WebTestCase, one of which is only
meant for internal tests. To avoid confusion the internal class has
been renamed to AbstractWebTestCase and made abstract.

See symfony#32577
janvt added a commit to janvt/symfony that referenced this issue Jul 19, 2019
@janvt
[FrameworkBundle] Rename internal WebTestCase to AbstractWebTestCase
bd4d2d9 
FrameworkBundle had 2 classes called WebTestCase, one of which is only
meant for internal tests. To avoid confusion the internal class has
been renamed to AbstractWebTestCase and made abstract.

See symfony#32577
janvt added a commit to janvt/symfony that referenced this issue Jul 19, 2019
@janvt
[SecurityBundle] Rename internal WebTestCase to AbstractWebTestCase
0c60b87 
SecurityBundle had 2 classes called WebTestCase, one of which is only
meant for internal tests. To avoid confusion the internal class has
been renamed to AbstractWebTestCase and made abstract.

See symfony#32577
@janvt janvt mentioned this issue Jul 19, 2019
Merged
janvt added a commit to janvt/symfony that referenced this issue Jul 19, 2019
@janvt
[FrameworkBundle] Rename internal WebTestCase to AbstractWebTestCase
5d11d21 
FrameworkBundle had 2 classes called WebTestCase, one of which is only
meant for internal tests. To avoid confusion the internal class has
been renamed to AbstractWebTestCase and made abstract.

See symfony#32577
janvt added a commit to janvt/symfony that referenced this issue Jul 19, 2019
@janvt
[SecurityBundle] Rename internal WebTestCase to AbstractWebTestCase
e02d313 
SecurityBundle had 2 classes called WebTestCase, one of which is only
meant for internal tests. To avoid confusion the internal class has
been renamed to AbstractWebTestCase and made abstract.

See symfony#32577
@janvt janvt mentioned this issue Jul 19, 2019
Merged
@Tobion
Copy link
Member

Tobion commented Jul 20, 2019

Can we reconsider #20057 please? WebTestCase is just one of the problems. There are alot of fixtures in tests that can easily be picked up by accident. And even the test files itself can cause confusion. If you look for WebTestCase you will also get suggested WebTestCaseTest. It's the test testing WebTestCase. Definitely not what you were looking for.

nicolas-grekas added a commit that referenced this issue Jul 23, 2019
@nicolas-grekas
minor #32625 [FrameworkBundle] [SecurityBundle] Rename internal WebTe…
e45c7a3 
…stCase to avoid confusion (janvt)

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

Discussion
----------

[FrameworkBundle] [SecurityBundle] Rename internal WebTestCase to avoid confusion

FrameworkBundle & SecurityBundle each had 2 classes called WebTestCase,
one of which is only meant for internal tests. To avoid confusion the internal
class has been renamed to AbstractWebTestCase and made abstract.

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no (or, yes, but internal class)
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #32577
| License       | MIT

This PR is to ease integration, as not all test classes are present in all currently
maintained branches.

See #32617

Commits
-------

775d970 [FrameworkBundle] [SecurityBundle] Rename internal WebTestCase to avoid confusion
janvt added a commit to janvt/symfony that referenced this issue Jul 23, 2019
@janvt
[FrameworkBundle] Rename internal WebTestCase to AbstractWebTestCase
63e3153 
FrameworkBundle had 2 classes called WebTestCase, one of which is only
meant for internal tests. To avoid confusion the internal class has
been renamed to AbstractWebTestCase and made abstract.

See symfony#32577
janvt added a commit to janvt/symfony that referenced this issue Jul 23, 2019
@janvt
[SecurityBundle] Rename internal WebTestCase to AbstractWebTestCase
388bdcb 
SecurityBundle had 2 classes called WebTestCase, one of which is only
meant for internal tests. To avoid confusion the internal class has
been renamed to AbstractWebTestCase and made abstract.

See symfony#32577
nicolas-grekas added a commit that referenced this issue Jul 23, 2019
@nicolas-grekas
minor #32626 [FrameworkBundle] [SecurityBundle] Rename internal WebTe…
9451217 
…stCase to avoid confusion (janvt)

This PR was squashed before being merged into the 4.2 branch (closes #32626).

Discussion
----------

[FrameworkBundle] [SecurityBundle] Rename internal WebTestCase to avoid confusion

FrameworkBundle & SecurityBundle each had 2 classes called WebTestCase,
one of which is only meant for internal tests. To avoid confusion the internal
class has been renamed to AbstractWebTestCase and made abstract.

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no (or, yes, but internal class)
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #32577
| License       | MIT

This PR is to ease integration, as not all test classes are present in all currently
maintained branches. This PR should patch relatively well onto 4.* branches.

See #32617

Commits
-------

01aaece [FrameworkBundle] [SecurityBundle] Rename internal WebTestCase to avoid confusion
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
DX DX = Developer eXperience (anything that improves the experience of using Symfony) FrameworkBundle Good first issue Ideal for your first contribution! (some Symfony experience may be required)
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@fabpot @javiereguiluz @weaverryan @nicolas-grekas @Tobion @ro0NL @xabbuh @janvt