Skip to content

[FrameworkBundle] Fix deps=low/high tests #16145

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
Oct 6, 2015
Merged

Conversation

nicolas-grekas
Copy link
Member

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

@@ -196,8 +196,13 @@ public function testTranslator()
'->registerTranslatorConfiguration() finds Form translation resources'
);
$ref = new \ReflectionClass('Symfony\Component\Security\Core\SecurityContext');
$ref = dirname($ref->getFileName());
if (!file_exists($ref.'/composer.json')) {
$this->assertFalse(file_exists($ref.'/Resources/translations/'));
Copy link
Member Author

Choose a reason for hiding this comment

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

This one ensures that we won't merge in Core/Resources/translations again (should be removed on 2.7)

Copy link
Member

Choose a reason for hiding this comment

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

This test should rather be in the security component (otherwise you cannot remove this in 2.7 because of the 2.7 low deps).

And then, in 2.7+, it should be replaced by a test ensuring they are in sync (or at least than the new files don't miss any translations from the old file).

Copy link
Member

Choose a reason for hiding this comment

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

see #16146

Copy link
Member Author

Choose a reason for hiding this comment

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

too much crack today for me :) updated!

@@ -89,4 +89,9 @@ public function testGetSetToken()
$context->setToken($token = $this->getMock('Symfony\Component\Security\Core\Authentication\Token\TokenInterface'));
$this->assertSame($token, $context->getToken());
}

public function testResourcesAreNotInCore()
Copy link
Member

Choose a reason for hiding this comment

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

I would say Translations, not Resources

Copy link
Member Author

Choose a reason for hiding this comment

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

renamed

@stof
Copy link
Member

stof commented Oct 6, 2015

👍

Status; reviewed

@fabpot
Copy link
Member

fabpot commented Oct 6, 2015

Thank you @nicolas-grekas.

@fabpot fabpot merged commit 26ca3dc into symfony:2.3 Oct 6, 2015
fabpot added a commit that referenced this pull request Oct 6, 2015
This PR was merged into the 2.3 branch.

Discussion
----------

[FrameworkBundle] Fix deps=low/high tests

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

Commits
-------

26ca3dc [FrameworkBundle] Fix deps=low/high tests
@nicolas-grekas nicolas-grekas deleted the fix branch October 6, 2015 15:49
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.

5 participants