SolomonDefi / solomon-monorepo

Monorepo containing core Solomon apps, services, libraries, and deploy config.
6 stars 3 forks source link

Add E2E test for mailgun #32

Open apolkingg8 opened 3 years ago

apolkingg8 commented 3 years ago

Fork from #17. Currently, we do some simple unit test to ensure the send() method is triggered, but the send() method is not tested yet. Need a mailgun test env to impl the E2E test.

beefho67 commented 2 years ago

@kelvin-wong Please share your past experience about this kind of e2e test.

kelvin-wong commented 2 years ago

To simplify the e2e test for mailgun, there is a project that works well to mock the service. We could include this service in our project for e2e testing.

beefho67 commented 2 years ago

@kelvin-wong Please share your past experience about this kind of e2e test.

  • It depends on Dockerfile of mail gun issue.

Hi @solomondefi-dev I don't find the Dockerfile of mail gun issue. Can you share the issue number? Thanks.

solomondefi-dev commented 2 years ago

You're right, I was thinking of #22

We'll need a new issue for the mail tester docker file

apolkingg8 commented 2 years ago

Just take a look at the Go project. If we don't need to test mailgun itself, this issue is pretty easy. We can add use a simple SQLite in the e2e test and don't need to import the full project into Solomon.

beefho67 commented 2 years ago

Just take a look at the Go project. If we don't need to test mailgun itself, this issue is pretty easy. We can add use a simple SQLite in the e2e test and don't need to import the full project into Solomon.

Hi @apolkingg8, In that case, does this issue still depend on #22? I think you are already working on this issue now.

apolkingg8 commented 2 years ago

Not sure. I don't think we need a Dockerfile in this case, but it's still ok to have one.

solomondefi-dev commented 2 years ago

We can add use a simple SQLite in the e2e test and don't need to import the full project into Solomon.

If I understand correctly, this is probably fine for integration tests. Since e2e tests should be run against skaffold and match production as close as possible, it could be nice to include an image for the mail tester in the test cluster, so we don't have to mock anything.

It should be really fast, either we can prebuild an image and upload to Github/Docker cloud, or have a simple Dockerfile that loads it from the github reposity and builds.

apolkingg8 commented 2 years ago

If I understand correctly, this is probably fine for integration tests. Since e2e tests should be run against skaffold and match production as close as possible, it could be nice to include an image for the mail tester in the test cluster, so we don't have to mock anything.

But about the exist project, it's like a mailgun mock, isn't it? If we want to match production as close as possible, that means we need to test "the mail is in the inbox", that's hard work. If we trust mailgun service and just check we send correct args to mailgun, it's should be done in the unit test and we don't need this issue.

solomondefi-dev commented 2 years ago

I do trust mailgun, but they unfortunately do not provide a good sandbox for testing, it's pretty limited. The mail tester has a few benefits over mocking locally:

It seems that since the mail-tester project was created, Mailgun themselves have built something similar: https://github.com/mailgun/mailgun-go/blob/master/mock.go

That appears to have many more features, but it's also a lot more complicated and I think it doesn't have a local mail viewer for development.