Fix running tests for autoloaded classes #1327
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Tests failed. Not sure if they were passing before this code change. Also please add a test to prove that it really works. |
|
Too fast, @aik099! I was just typing exactly that ;) PS: The tests were indeed passing before the change. |
|
They pass now =) |
|
@dracony Would you mind adding new tests for the bug this PR fixes? |
|
Added tests. Please merge this as I really need this functionality to be finally working =P thanks |
|
Can this pleeeeeeease be merged now? I added tests, it basses build. Why does it take so looong? |
|
@dracony Please be patient. Your PR has possible BC and performance implications that I need to investigate before I can merge it. I hope to have time this weekend, but no guarantees. |
| $baseName = str_replace('.php', '', basename($filename)); | ||
|
|
||
| foreach ($newClasses as $className) { | ||
| foreach ($this->untestedClasses as $className) { | ||
| if (substr($className, 0 - strlen($baseName)) == $baseName) { | ||
| $class = new ReflectionClass($className); |
There was a problem hiding this comment.
I think you should remove classes that have been loaded from $untestedClasses, i.e. something like
foreach ($this->untestedClasses as $i => $className) {
if (substr(...)) {
...
unset($this->untestedClasses[$i]);
}
}
|
I second this PR. |
|
@dracony This looks great. Sorry it took me so long to get around to it. Would you mind opening a new PR that targets the 4.1 branch so that we could get this in the next stable release. Also, I think the suggestion from @webmozart makes sense. Would you mind incorporating that? |
|
Done! Here is the pull request: #1362 |
|
@dracony In the future, I recommend to create separate branches for PRs (e.g. |
webmozart
added a commit
to symfony/symfony
that referenced
this pull request
Aug 4, 2014
…ebmozart) This PR was squashed before being merged into the 2.5 branch (closes #11485). Discussion ---------- [Validator] Constraint validators now use the 2.5 API | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - See the comments in #11049 for the origin of this PR. Currently, the 2.5 API needs to use `LegacyExecutionContextFactory` because the constraint validators rely on methods from the old `ExecutionContext` class (like `validate()`, `validateValue()`). Consequently it is impossible to switch to the pure 2.5 API and check whether all calls to deprecated methods were removed from application code. This is fixed now. This PR also introduces a complete test suite to test each constraint validator against all three APIs: 2.4, 2.5-BC and 2.5. Currently, some tests are not executed yet when running the complete test suite is run. I expect this to be fixed soon (ticket: sebastianbergmann/phpunit#529, pr: sebastianbergmann/phpunit#1327). Commits ------- 295e5bb [Validator] Fixed failing tests 3bd6d80 [Validator] CS fixes 870a41a [FrameworkBundle] Made ConstraintValidatorFactory aware of the legacy validators 7504448 [Validator] Added extensive test coverage for the constraint validators for the different APIs 8e461af [Validator] Constraint validators now use the 2.5 API. For incompatible validators, legacy validators were created
symfony-splitter
pushed a commit
to symfony/validator
that referenced
this pull request
Aug 4, 2014
…ebmozart) This PR was squashed before being merged into the 2.5 branch (closes #11485). Discussion ---------- [Validator] Constraint validators now use the 2.5 API | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - See the comments in #11049 for the origin of this PR. Currently, the 2.5 API needs to use `LegacyExecutionContextFactory` because the constraint validators rely on methods from the old `ExecutionContext` class (like `validate()`, `validateValue()`). Consequently it is impossible to switch to the pure 2.5 API and check whether all calls to deprecated methods were removed from application code. This is fixed now. This PR also introduces a complete test suite to test each constraint validator against all three APIs: 2.4, 2.5-BC and 2.5. Currently, some tests are not executed yet when running the complete test suite is run. I expect this to be fixed soon (ticket: sebastianbergmann/phpunit#529, pr: sebastianbergmann/phpunit#1327). Commits ------- 295e5bb [Validator] Fixed failing tests 3bd6d80 [Validator] CS fixes 870a41a [FrameworkBundle] Made ConstraintValidatorFactory aware of the legacy validators 7504448 [Validator] Added extensive test coverage for the constraint validators for the different APIs 8e461af [Validator] Constraint validators now use the 2.5 API. For incompatible validators, legacy validators were created
fago
pushed a commit
to fago/Validator
that referenced
this pull request
Apr 18, 2015
…ebmozart) This PR was squashed before being merged into the 2.5 branch (closes #11485). Discussion ---------- [Validator] Constraint validators now use the 2.5 API | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - See the comments in #11049 for the origin of this PR. Currently, the 2.5 API needs to use `LegacyExecutionContextFactory` because the constraint validators rely on methods from the old `ExecutionContext` class (like `validate()`, `validateValue()`). Consequently it is impossible to switch to the pure 2.5 API and check whether all calls to deprecated methods were removed from application code. This is fixed now. This PR also introduces a complete test suite to test each constraint validator against all three APIs: 2.4, 2.5-BC and 2.5. Currently, some tests are not executed yet when running the complete test suite is run. I expect this to be fixed soon (ticket: sebastianbergmann/phpunit#529, pr: sebastianbergmann/phpunit#1327). Commits ------- 295e5bb [Validator] Fixed failing tests 3bd6d80 [Validator] CS fixes 870a41a [FrameworkBundle] Made ConstraintValidatorFactory aware of the legacy validators 7504448 [Validator] Added extensive test coverage for the constraint validators for the different APIs 8e461af [Validator] Constraint validators now use the 2.5 API. For incompatible validators, legacy validators were created
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Should fix this issue: #529
This will always perform the check for $suiteClassName inside loaded classes