Skip to content

CI fails because of forwards compatibility deprecations #33483

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

Closed
derrabus opened this issue Sep 5, 2019 · 1 comment
Closed

CI fails because of forwards compatibility deprecations #33483

derrabus opened this issue Sep 5, 2019 · 1 comment

Comments

@derrabus
Copy link
Member

derrabus commented Sep 5, 2019

Symfony version(s) affected: 4.4-dev

Description
#33430 taught DebugClassLoader to detect deprecated method signatures in classes that aren't autoloaded. Tests on the 4.4 branch are currently deep red because of this.

The feature is problematic because it also affects fixtures that intentionally have an old signature because we want to make sure that such classes can still be used.

  • Adding @group legacy to the test cases does not always help: If the fixture classes is defined in the same file as the test case, it is loaded before the test is even executed. Of course, we could move the class to a dedicated php file, but…
  • Those fixture classes are usually used for tests that already test for a deprecation message. Adding another one isn't helpful in that case.

How to reproduce
https://travis-ci.org/symfony/symfony/jobs/581278283

Possible Solution

  • It is in most cases possible to work around the issue by using an anonymous class as fixture. DebugClassLoader is not (yet?) able to check those. But that feels like a dirty workaround.
  • Maybe we can come up with a way to way to mark those classes so that DebugClassLoader will skip checks on them.

ping @nicolas-grekas

@derrabus derrabus changed the title Tests fail because of forwards compatibility deprecations CI fails because of forwards compatibility deprecations Sep 6, 2019
nicolas-grekas added a commit that referenced this issue Sep 6, 2019
…rabus, nicolas-grekas)

This PR was merged into the 4.3 branch.

Discussion
----------

Fix test fixtures with deprecated method signatures

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

This PR upgrades two fixtures that implemented deprecated method signatures. As far as I can tell, they are used in tests that do not specifically test legacy behavior, so the fixtures should be up to date. Currently, these fixtures cause failing tests on the 4.4 branch.

Commits
-------

cc3e3d5 Fix more bad tests
592aacf Fix test fixtures with deprecated method signatures.
@nicolas-grekas
Copy link
Member

nicolas-grekas commented Sep 6, 2019

Most cases were actually legit :)
The only non legits are the one of the Debug component, I'm on it.
Thanks for having a look!

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

No branches or pull requests

2 participants