Fix (1067): Allow Multiple Email Servers for Test Suites

[irby]

Description

This pull request resolves an issue described in PR #1067 , which will help resolve issue blocking #1030. The crux of the issue is that the current EmailServer solution is run per test suite. If multiple tests rely on an email server, it will attempt to create multiple instances of a mock SMTP server and will fail the test suite if these tests overlap. There isn't a clean way to get around this issue, even forcing tests to run non-parallel does not help, nor does ignoring errors where an existing server is already running.

As a solution, this code change will start a Dockerized Mailslurper instance per test suite instance. Mailslurper offers an API solution so we can easily query emails and their content.

Implementation

• Similar to how test suites setup a Dockerized DB, this solution will setup a Mailslurper instance using dockertest.
• After each test is complete, emails are deleted from the mail server.

[lfleischmann]

Hi @irby,

are you aware that the CI/the tests are failing?

[irby]

Hi @FlxMgdnz, yes. I could not reproduce this issue locally; I opened a PR against my own fork and the Go test did not fail there. I’ll continue looking to figure out why this is failing.

[FlxMgdnz]

Cc @lfleischmann

[lfleischmann]

Hi @irby ,

the tests don't run for me locally either (passcode handler test fails, run via go test ./... -v). My guess is that I need to start a container locally now?

[irby]

Hi @lfleischmann, yes, you'll need to start a container for Mailslurper by using the command docker-compose -f ./backend/test/docker-compose.test.yaml up --build -d prior to running tests.

I've added documentation to the backend README on how to run these tests.

[irby]

I believe I've discovered the root cause here. I was able to reproduce this issue locally after a couple runs. It's an error that happens occasionally, but not on every run.

Because these tests are running in parallel, I believe one or more tests is (occasionally) interfering with the HTTP request to get emails from the test mailslurper instance.

Based on the error logs in the failed run, I see that the exception was coming from Gock, a mock HTTP interceptor that's use in the third party callback code.

I'm not familiar with Gock, so I don't know the best path forward, but these are my thoughts on the workaround here:

• We make the passcode_test suite non-parallel
• At the start of the TestPasscodeHandler_Init, we call gock.Off to ensure no HTTP interception will interfere with our Mailslurper HTTP calls.

After several tests locally, I'm seeing this with reliably passing results.

Let me know what you think.

[lfleischmann]

I'm not familiar with Gock, so I don't know the best path forward, but these are my thoughts on the workaround here:

• We make the passcode_test suite non-parallel
• At the start of the TestPasscodeHandler_Init, we call gock.Off to ensure no HTTP interception will interfere with our Mailslurper HTTP calls.

After several tests locally, I'm seeing this with reliably passing results.

Would this remove the requirement for the mail slurper container again? We talked about this internally and we're not 100% happy with the requirement of having to run another container manually before running tests.

But: we already use dockertest in our/one of our test suites to spin up a database along with testfixtures. Maybe it would be possible to adjust/extend the suite with the possibility of spinning up a mailslurper just like with the db? You already edited the suite, so maybe this would be an appropriate point to do that?

[irby]

@lfleischmann I'll certainly take a look into it. I was curious how those test suite Docker instances were being setup. I think it would be better to have the test suite be responsible for setting up the containers so we don't have the overhead of spinning up the docker before running tests.

[irby]

@lfleischmann After using the solution you've posted, I like the solution much better. My setup is that if a suite has its IncludeEmailServer property is set to true, it will spin up a Mailslurper container specific to that test suite.

It's working quite well, but I'm noticing that every so often a test fails because the emails we receive from Mailslurper's API is a little out of sync with what's been sent. I'm looking into solutions to resolve this.

[lfleischmann]

@irby

It's working quite well, but I'm noticing that every so often a test fails because the emails we receive from Mailslurper's API is a little out of sync with what's been sent. I'm looking into solutions to resolve this.

Not sure if it is the same as your "out of sync" experience but: I had to give it some leeway in the e2e tests too because it looked like inbound messages were not processed "fast enough".

[irby]

@lfleischmann

Not sure if it is the same as your "out of sync" experience but: I had to give it some leeway in the e2e tests too because it looked like inbound messages were not processed "fast enough".

I was going to do something similar, as much as I don't like adding in pauses in tests, but when tests reach out to the Mailslurper API to fetch emails, I'm adding in a delay of a few milliseconds before making that call. Currently, it's a 15 ms delay. So far, I've run 30 tests without issue. I'll refactor my changes a little bit, run some more tests, and push a change up soon.

[irby]

@lfleischmann This is ready for review.

[lfleischmann]

LGTM now, thanks!