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 chapter to mailer #16195

Merged
merged 1 commit into from Dec 8, 2021
Merged

Add functional test chapter to mailer #16195

merged 1 commit into from Dec 8, 2021

Conversation

famoser
Copy link
Contributor

@famoser famoser commented Dec 5, 2021

Formulation largely from https://symfony.com/doc/4.4/email.html#how-to-test-that-an-email-is-sent-in-a-functional-test
Simple example which showcases the "global" methods like assertEmailCount and the e-mail specific assertions.
Taken from actual code in use, so should be close to what user wants to accomplish.

Targets the 4.4 branch as described in the docs.
Resolves #14397

@carsonbot carsonbot added this to the 4.4 milestone Dec 5, 2021
@famoser
Copy link
Contributor Author

famoser commented Dec 5, 2021

The failing build is due to failing:

git clone -b 4.4 --depth 5 --single-branch https://github.com/symfony-tools/symfony-application.git _sf_app
cd _sf_app/
composer update

The problem is that composer update installs symfony@6.0 but then the deprecated Symfony\Component\Routing\RouteCollectionBuilder is still used in src/Kernel.php. Note that https://github.com/symfony-tools/symfony-application/blob/4.4/composer.json uses wildcards for symfony dependencies.

@famoser
Copy link
Contributor Author

famoser commented Dec 5, 2021

This can be solved either here or upstream.

Here

composer update --no-scripts # prevent the failing cache:clear command to run
composer recipes:install symfony/framework-bundle --force # path Kernel.php
rm -rf vendor/ # remove deps again
composer update # now proceed as before

(note: not removing the vendor directory downgrades the dependencies back to ^4.4. Not seeing why though at the moment.)

Upstream

I think it would be cleaner to solve it upstream: Specify the symfony dependency in composer json: https://github.com/symfony-tools/symfony-application/blob/4.4/composer.json. For example, specify "symfony/framework-bundle": "^4.4" on branch 4.4. I'll ping the author @Nyholm who probably knows why it is not done that way at the moment.

@Nyholm
Copy link
Member

Nyholm commented Dec 6, 2021

You are correct. Branch 4.4 on https://github.com/symfony-tools/symfony-application should install only version 4.4 of all symfony packages. Feel free to prepare a PR and I'll make sure to fix this ASAP.

@Nyholm
Copy link
Member

Nyholm commented Dec 8, 2021

Could we rerun the CI?

Either docs team restart it or @famoser runs:

git commit --allow-empty  -m "Trigger CI" 
git push

@famoser
Copy link
Contributor Author

famoser commented Dec 8, 2021

Done, thanks for your help!

I see that all PR from the last three days look stuck from the same issue we have just fixed. It might be quicker if the doc team could restart for all of them, rather than each one makes its own empty commit.

@Nyholm
Copy link
Member

Nyholm commented Dec 8, 2021

I've notified the doc team.

Thank you for testing and confirming that it works.

Copy link
Member

@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

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

I like this PR. Well done!

There is just some syntax fixes.

PS. I know that if we asks the docs team nicely they will do syntax fixes for us.

mailer.rst Outdated Show resolved Hide resolved
Copy link
Member

@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

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

Awesome!
Thank you @famoser for making this PR and to debugging and fixing the CI.

@OskarStark
Copy link
Contributor

OskarStark commented Dec 8, 2021

Thank you very much, I appreciate your contribution and congratulations on your first contribution to the Symfony documentation 🎉 I made some minor adjustements while merging your PR 👍

@OskarStark OskarStark merged commit fcdcae8 into symfony:4.4 Dec 8, 2021
@Nyholm
Copy link
Member

Nyholm commented Dec 8, 2021

Wohoo!

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.

None yet

4 participants