8000 [2.7] Fix tests by paradajozsef · Pull Request #17582 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[2.7] Fix tests #17582

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

Closed
wants to merge 9 commits into from
Closed

[2.7] Fix tests #17582

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
26cc962
[FrameworkBundle] Fix frameworkextension test
paradajozsef Jan 27, 2016
3ed3c4c
[FrameworkBundle] Frameworkextension test fixed
paradajozsef Jan 27, 2016
d9587b3
[FrameworkBundle] Frameworkextension test fix
paradajozsef Jan 28, 2016
b7376c0
[FrameworkBundle] Frameworkbundle test fix, remove dump
paradajozsef Jan 28, 2016
5d83c54
[FrameworkBundle] Move ::fragment.html.php to Resources/views dir
paradajozsef Jan 28, 2016
8094d0a
[FrameworkBundle] fabbot.io fix
paradajozsef Jan 28, 2016
d1f1872
Fix: apcu_clear_cache() expects exactly 0 parameters
paradajozsef Jan 28, 2016
10ab042
[FrameworkBundle] Frameworkextension test fix
paradajozsef Jan 28, 2016
3c3f8d2
[FrameworkBundle] Test fix revert
paradajozsef Jan 30, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
[FrameworkBundle] Frameworkextension test fixed
  • Loading branch information
@paradajozsef
paradajozsef committed Jan 27, 2016
commit 3ed3c4cf5578d30ba34d87fb0886793dcc189edc
2 changes: 1 addition & 1 deletion src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,7 @@ private function checkAssetsPackages(ContainerBuilder $container, $legacy = fals
$package = $container->getDefinition($packages['bar']);
$this->assertUrlPackage($container, $package, array('https://bar2.example.com'), $legacy ? null : 'SomeVersionScheme', $legacy ? '%%s?%%s' : '%%s?version=%%s');

$this->assertEquals($legacy ? 'assets.empty_version_strategy' : 'assets._version__default', (string) $container->getDefinition('assets._package_bar')->getArgument(1));
$this->assertEquals('assets.empty_version_strategy', (string) $container->getDefinition('assets._package_bar')->getArgument(1));
Copy link
Member

Choose a reason for hiding this comment

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

This is a BC break. Before it was ensured that the version strategy following the new configuration options (the ones introduced with Symfony 2.7) was the default version strategy when one omitted them in their config. This now changes it to the empty version strategy (well, not this change but this change to the test ensures that we agree with that break).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see.

This behaviour was introduced in this commit

If I'm right, the problem is with this condition change, because $package['version'] key is always exists, since it has a null default value. So $version never will be equals with $defaultVersion (which is a Reference with an id of 'assets._version__default')

Copy link
Member

Choose a reason for hiding this comment

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

I think adding null as the default value here was a mistake. I opened #17605 which proposes to revert that change (that would make the Asset related tests from the FrameworkBundle pass again).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice! I revert changes I made for FrameworkExtensionTest and then this PR fixes the remaining failing tests.

$this->assertEquals('assets.empty_version_strategy', (string) $container->getDefinition('assets._package_bar_null_version')->getArgument(1));
}

Expand Down
0