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

Add functional test and fix container usage #472

Merged
merged 4 commits into from
Jan 25, 2022

Conversation

franmomu
Copy link
Member

@franmomu franmomu commented Jan 16, 2022

Subject

I was trying to add a functional test and in the way some issues came up.

I am targeting this branch, because this is BC.

Closes #471.

Changelog

### Deprecated
- Constructing `TokenStorageUsernameCallable` with an instance of `Container`, use an instance of `TokenStorageInterface` instead
### Fixed
- Fixed service id of `ViewEntityAction`
### Removed
- Support for `doctrine/orm` < 1.12.8

tests/custom_bootstrap.php Outdated Show resolved Hide resolved
tests/Functional/SmokeTest.php Outdated Show resolved Hide resolved
@VincentLanglet VincentLanglet requested review from jordisala1991 and a team January 16, 2022 17:47
* file that was distributed with this source code.
*/

namespace SimpleThings\EntityAudit\Tests\App;
Copy link
Member

Choose a reason for hiding this comment

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

This would be problematic for dev-kit, it wont be able to generate the correct namespace. It will try to use Sonata when adding the console lints.

Since this is tests files, we could change the test namespace to use Sonata and it would fix this problem. Or you will need to introduce an if in the code for this case

Copy link
Member Author

Choose a reason for hiding this comment

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

I've removed the container test in dev-kit sonata-project/dev-kit#1880 in the meantime.

Maybe the console bin file could read the AppKernel class name from an external variable or be able to configure it in dev-kit using as default name the current behaviour.

Copy link
Member

Choose a reason for hiding this comment

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

Our goal was stop remove the option from the dev-kit later, so it would be nice to not rely on a name provided by the devkit.

@jordisala1991 jordisala1991 merged commit 0175923 into sonata-project:1.x Jan 25, 2022
@jordisala1991
Copy link
Member

Thanks @franmomu

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

Successfully merging this pull request may close these issues.

The "security.token_storage" service or alias has been removed or inlined when the container was compiled.
3 participants