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

Turn some test helpers from macros -> functions #604

Merged
merged 2 commits into from
May 27, 2021

Conversation

germsvel
Copy link
Collaborator

What changed?

We change three test helpers in Bamboo.Test from macros into regular functions:

  • assert_delivered_email/1
  • assert_delivered_email_with/1
  • refute_email_delivered_with/1

There are other helpers that are already functions:

  • assert_no_emails_delivered/0
  • refute_delivered_email/1

The only macro we leave as a macro (because it needs to be one) is assert_delivered_email_matches/1

Why?

There seems no reason for those functions to be macros, and they only add extra complexity.

Macros should only be used as a last resort

A side benefit

A side benefit of doing this is that we can make all the private functions truly private with defp. With the macros, we had to make them public (to access them from the macros), and we set @doc false so they wouldn't show up in docs.

lib/bamboo/test.ex Outdated Show resolved Hide resolved
@germsvel germsvel force-pushed the regular-function-test-helpers branch from aae302c to 3faf9ef Compare May 27, 2021 09:38
What changed?
============

We change three test helpers in `Bamboo.Test` from macros into regular
functions:

- `assert_delivered_email/1`
- `assert_delivered_email_with/1`
- `refute_email_delivered_with/1`

There are other helpers that are already functions:

- `assert_no_emails_delivered/0`
- `refute_delivered_email/1`

The only macro we leave as a macro (because it needs to be one) is
`assert_delivered_email_matches/1`

Why?
----

There seems no reason for those functions to be macros, and they only
add extra complexity.

> Macros should only be used as a last resort

- from Elixir's [getting started
  docs](https://elixir-lang.org/getting-started/meta/macros.html)

A side benefit
-------------

A side benefit of doing this is that we can make most of the private
functions truly private with `defp`. With the macros, we had to make
them public (to access them from the macros), and we set `@doc false` so
they wouldn't show up in docs.
@germsvel germsvel force-pushed the regular-function-test-helpers branch from 3faf9ef to ae224b1 Compare May 27, 2021 10:43
Copy link
Contributor

@brian-penguin brian-penguin left a comment

Choose a reason for hiding this comment

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

👍 looks nice and clean

@germsvel germsvel merged commit 6aa74c3 into master May 27, 2021
@germsvel germsvel deleted the regular-function-test-helpers branch May 27, 2021 20:37
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.

2 participants