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

DummyMailer.send method does not return success value like Mailer.send, causing inconsistencies in tests #101

Open
michaellaunay opened this issue Oct 8, 2024 · 0 comments

Comments

@michaellaunay
Copy link

Hello,

I have encountered an issue with the DummyMailer class in pyramid_mailer. The send method of DummyMailer does not return a value, whereas the send method of the actual Mailer class returns a success value (e.g., True). This discrepancy can cause problems in unit tests where the code under test relies on the return value of send to determine if an email was sent successfully.

Details:

In the Mailer class, the send method is defined as follows:

def send(self, message):
    """Send a message.

    The message is handled inside a transaction, so in case of failure
    (or the message fails) the message will not be sent.

    :param message: a 'Message' instance.
    """
    return self.direct_delivery.send(*self._message_args(message))

As you can see, it returns the result of self.direct_delivery.send, which typically indicates the success of the operation.

However, in the DummyMailer class used for testing, the send method does not return any value:

class DummyMailer(object):
    # ...

    def send(self, message):
        """Mock sending a transactional message via SMTP.

        The message is appended to the 'outbox' list.

        :param message: a 'Message' instance.
        """
        self.outbox.append(message)
        # No return value here

This difference leads to issues in tests where the application code depends on the return value of send. For example:

result = request.registry['mailer'].send(message)
if not result:
    # Handle the failure case
    pass

In this scenario, since DummyMailer.send returns None, the condition not result evaluates to True, and the code incorrectly assumes that the email sending failed, even though it was added to the outbox.

Proposed Solution:

Modify the DummyMailer.send method to return a success value, consistent with the Mailer.send method. For instance:

class DummyMailer(object):
    # ...

    def send(self, message):
        self.outbox.append(message)
        return True  # Indicate success

This change would ensure that tests using DummyMailer behave consistently with the actual mailer, and prevent false negatives in test assertions.

Additional Considerations:

  • The same issue may exist with other methods in DummyMailer, such as send_immediately, send_to_queue, etc. It would be beneficial to review these methods to ensure they also return appropriate success values if the real Mailer methods do.

  • Ensuring that DummyMailer mimics the behavior of Mailer as closely as possible is crucial for accurate testing and reliable test results.

Environment:

  • pyramid_mailer version: [Please specify]
  • Python version: [Please specify]
  • Operating system: [Please specify]

Steps to Reproduce:

  1. Write application code that relies on the return value of mailer.send to determine success.
  2. In unit tests, use DummyMailer from pyramid_mailer.testing.
  3. Observe that mailer.send returns None during tests, causing the application code to misinterpret the result.

Expected Behavior:

The DummyMailer.send method should return a value indicating success (e.g., True), matching the behavior of the real Mailer.send method.

Actual Behavior:

DummyMailer.send does not return any value, resulting in None, which can be misinterpreted as a failure in application code that relies on the return value.

Possible Workarounds:

  • Manually patching DummyMailer in tests to return True.
  • Modifying application code to not rely on the return value of send, although this may not be feasible in all cases.

Request:

I kindly request that the DummyMailer class be updated so that its methods return values consistent with the Mailer class methods. This change would improve the reliability of tests and ensure consistent behavior between testing and production environments.

Thank you for your attention to this matter.

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

No branches or pull requests

1 participant