Navigation Menu

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

compatibility with phpunit8 #30474

Merged
merged 1 commit into from Mar 9, 2019
Merged

compatibility with phpunit8 #30474

merged 1 commit into from Mar 9, 2019

Conversation

garak
Copy link
Contributor

@garak garak commented Mar 7, 2019

This basically adds the same phpunit8 compatibility layer added in #30084 (but for other test classes)
See also discussion in #30071

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

@garak garak changed the title compatibnility with phpunit8 compatibility with phpunit8 Mar 7, 2019
@nicolas-grekas nicolas-grekas added this to the 3.4 milestone Mar 7, 2019
@nicolas-grekas
Copy link
Member

What about a generic trait that would call a private doSetUp() method?

@garak
Copy link
Contributor Author

garak commented Mar 7, 2019

What about a generic trait that would call a private doSetUp() method?

You mean a single trait with all methods for the 3 classes?

@nicolas-grekas
Copy link
Member

yes, and keep the code in the current classes

@garak
Copy link
Contributor Author

garak commented Mar 7, 2019

Well, it makes sense.
But I'm not sure about the place where to put such unique trait, being the 3 classes spread in 2 components (Form and Validator)

@nicolas-grekas
Copy link
Member

🤔 3 proposals: one trait per component, add the trait in the bridge, add the trait in contracts. Pick one :)

@garak
Copy link
Contributor Author

garak commented Mar 7, 2019

Better now? Let me know, I'll squash

@nicolas-grekas
Copy link
Member

Looks like we're converging on something nice :)

Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

Looks nice, thanks!
What about adding the same trait - not internal this time - on the bridge in 4.3 (master)?

@fabpot
Copy link
Member

fabpot commented Mar 9, 2019

Thank you @garak.

@fabpot fabpot merged commit 5ef254f into symfony:3.4 Mar 9, 2019
fabpot added a commit that referenced this pull request Mar 9, 2019
This PR was merged into the 3.4 branch.

Discussion
----------

compatibility with phpunit8

This basically adds the same phpunit8 compatibility layer added in #30084 (but for other test classes)
See also discussion in #30071

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

Commits
-------

5ef254f compatibility with phpunit8
fabpot added a commit that referenced this pull request Mar 19, 2019
…Dude)

This PR was merged into the 3.4 branch.

Discussion
----------

[Tests] fixed compatbility of assertEquals(): void

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

Same as #30474.

Commits
-------

3f7bedc [Tests] fixed compatbility of assertEquals(): void
This was referenced Apr 2, 2019
@garak garak deleted the phpunit8-compat-test branch March 27, 2020 16:58
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