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 "Writing a custom Behat formatter" cookbook #149

Open
wants to merge 3 commits into
base: v3.0
Choose a base branch
from

Conversation

jdeniau
Copy link
Contributor

@jdeniau jdeniau commented Oct 29, 2024

Fixes #143

Copy link
Contributor

@acoulton acoulton left a comment

Choose a reason for hiding this comment

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

@jdeniau thanks very much for this, it's a great addition to the documentation.

I've made some suggestions for style / grammar things that it would be good to fix - I feel bad picking them out, as there's no way I could write anything like this in French!

If you're OK with my suggestions I'd be happy to apply them for you.

cookbooks/custom_formatter.rst Outdated Show resolved Hide resolved
cookbooks/custom_formatter.rst Outdated Show resolved Hide resolved
cookbooks/custom_formatter.rst Outdated Show resolved Hide resolved
cookbooks/custom_formatter.rst Outdated Show resolved Hide resolved
cookbooks/custom_formatter.rst Outdated Show resolved Hide resolved
cookbooks/custom_formatter.rst Outdated Show resolved Hide resolved
cookbooks/custom_formatter.rst Outdated Show resolved Hide resolved
cookbooks/custom_formatter.rst Outdated Show resolved Hide resolved
cookbooks/custom_formatter.rst Outdated Show resolved Hide resolved
cookbooks/custom_formatter.rst Outdated Show resolved Hide resolved
@jdeniau
Copy link
Contributor Author

jdeniau commented Nov 1, 2024

@acoulton do not feel bad, English is not my main language and I make a lot of errors. I make typos too 🙂

I'm OK with all your suggestions. I'm not on a computer right now, so if you want to make the changes yourself no problem for me. If not, I will commit this in the next days.

Copy link
Contributor

@acoulton acoulton left a comment

Choose a reason for hiding this comment

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

Thanks @jdeniau I've added those edits for you.

@acoulton acoulton requested a review from ttomdewit November 1, 2024 07:58
Copy link

@ttomdewit ttomdewit left a comment

Choose a reason for hiding this comment

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

It's a really good addition. The only changes I'd request would be to replace "behat" with "Behat" and try and stay away from terms like "simple" when trying to explain a step. It's not a "must solve" on my end, but something that's simple to a lot of people might not be simple for others.

@acoulton
Copy link
Contributor

acoulton commented Nov 1, 2024

Thanks @ttomdewit I missed some of the [bB]ehat. That's very good advice on "simple".

I'll take another pass later unless @jdeniau gets there first.

@acoulton acoulton force-pushed the behat-formatter-cookbook branch from 8223594 to fb03842 Compare November 1, 2024 09:24
@acoulton acoulton requested a review from ttomdewit November 1, 2024 09:25
@acoulton
Copy link
Contributor

acoulton commented Nov 1, 2024

@ttomdewit I pulled it into my IDE to find and replace all the lowercase behat, and the spellchecker also highlighted a couple of typos I hadn't seen myself so I've fixed them too 😆

- any stdin, coupled with an "errorformat" (a Vim inspired format that can convert text string to machine-readable errors),
- a `"Reviewdog Diagnostic Format" <https://github.com/reviewdog/reviewdog/tree/48b25a0aafb8494e751387e16f729faee9522c46/proto/rdf>`__: a JSON with error data that reviewdog can parse.

In my case, I tried parsing Behat's output with errorformat, but I do not know this language, and the multi-line Behat output with "dots" didn't make it easy.
Copy link
Member

Choose a reason for hiding this comment

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

I don't think the documentation should by using I here. this looks fine for a blog post (where I refers to the author of the post), but not so much for documentation.

*
* There are a lot of other events that can be found here in the Behat\Testwork\EventDispatcher\Event class
*/
public static function getSubscribedEvents()
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public static function getSubscribedEvents()
public static function getSubscribedEvents(): array

This way, this will be compatible with the next major version of Symfony which will make the return type mandatory.

Create the extension
~~~~~~~~~~~~~~~~~~~~

Any Behat extensions must implement ``Behat\Testwork\ServiceContainer\Extension``. Under the hood, it implements Symfony ``CompilerPass``.
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 remove the mention of CompilerPass here, especially as the extension in this example implements an empty process method (which is the only method related to CompilerPass)


declare(strict_types=1);

namespace JDeniau\BehatReviewdogFormatter;
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 suggest using a generic namespace instead of your own namespace here, as this is going to be rendered in the official Behat documentation. #146 used HelloWorld as namespace


default:
extensions:
JDeniau\BehatReviewdogFormatter\ReviewdogFormatterExtension: ~
Copy link
Member

Choose a reason for hiding this comment

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

if you put your extension class in a ServiceContainer subnamespace, with a name matching the last section of your namespace, this unlocks using the shortcut.


That's how you can write a basic custom Behat formatter!

If you have much more complex logic, and you need the formatter to be more dynamic, Behat do provide a FormatterFactory interface.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
If you have much more complex logic, and you need the formatter to be more dynamic, Behat do provide a FormatterFactory interface.
If you have much more complex logic, and you need the formatter to be more dynamic, Behat provides a FormatterFactory interface.

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

Successfully merging this pull request may close these issues.

Custom formatter extension documentation
4 participants