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 end-to-end sandbox support #91

Merged
merged 2 commits into from Sep 19, 2017

Conversation

Projects
None yet
4 participants
@chrismccord
Member

chrismccord commented Sep 16, 2017

@josevalim as discussed. I still need to polish docs on end-to-end testing, but please give it a look and let me know how you feel about the approach. Thanks!

@chrismccord chrismccord requested a review from josevalim Sep 16, 2017

Show outdated Hide outdated lib/phoenix_ecto.ex Outdated
@michalmuskala

This comment has been minimized.

Show comment
Hide comment
@michalmuskala

michalmuskala Sep 16, 2017

Member

If one route uses POST and another DELETE, maybe it would make sense to have them on just one route? Something like POST /sandbox for checkout and DELETE /sandbox for checkin?

Member

michalmuskala commented Sep 16, 2017

If one route uses POST and another DELETE, maybe it would make sense to have them on just one route? Something like POST /sandbox for checkout and DELETE /sandbox for checkin?

@josevalim

This comment has been minimized.

Show comment
Hide comment
@josevalim

josevalim Sep 16, 2017

Member

Thank you, it looks great to me! I have added mostly cosmetic details. We also need more docs, not only around the whole use case, but also on the added functions, especially checkin and checkout which is only required for external cases and if you are implementing your own endpoint.

Member

josevalim commented Sep 16, 2017

Thank you, it looks great to me! I have added mostly cosmetic details. We also need more docs, not only around the whole use case, but also on the added functions, especially checkin and checkout which is only required for external cases and if you are implementing your own endpoint.

@chrismccord

This comment has been minimized.

Show comment
Hide comment
@chrismccord

chrismccord Sep 19, 2017

Member

@michalmuskala big 👍 for the single path option:

        plug Phoenix.Ecto.SQL.Sandbox,
          at: "/sandbox",
          repo: MyApp.Repo

Thanks for the idea!

Member

chrismccord commented Sep 19, 2017

@michalmuskala big 👍 for the single path option:

        plug Phoenix.Ecto.SQL.Sandbox,
          at: "/sandbox",
          repo: MyApp.Repo

Thanks for the idea!

@chrismccord chrismccord merged commit 330d4cd into master Sep 19, 2017

0 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/travis-ci/push The Travis CI build is in progress
Details

@chrismccord chrismccord deleted the cm-end-to-end-sandbox branch Sep 19, 2017

@@ -23,6 +23,7 @@ defmodule PhoenixEcto.Mixfile do
def application do
[
mod: {Phoenix.Ecto, []},

This comment has been minimized.

@fertapric

fertapric Sep 25, 2017

Hi @chrismccord! Awesome work!

One question, is it OK to run Phoenix.Ecto.SQL.SandboxSupervisor in all the environments, including production? I wonder if it would be better to start the supervisor explicitly when running the tests. What do you think?

@fertapric

fertapric Sep 25, 2017

Hi @chrismccord! Awesome work!

One question, is it OK to run Phoenix.Ecto.SQL.SandboxSupervisor in all the environments, including production? I wonder if it would be better to start the supervisor explicitly when running the tests. What do you think?

This comment has been minimized.

@josevalim

josevalim Sep 25, 2017

Member

It is totally fine. A process that does nothing won't be of any harm.

@josevalim

josevalim Sep 25, 2017

Member

It is totally fine. A process that does nothing won't be of any harm.

This comment has been minimized.

@fertapric

fertapric Sep 25, 2017

Yeah, it won’t :)

My concerns were related to people observing their system in development and/or production and seeing processes especifically designed for testing. If I would see that, I would wonder what’s the value of MIX_ENV env var 😅

@fertapric

fertapric Sep 25, 2017

Yeah, it won’t :)

My concerns were related to people observing their system in development and/or production and seeing processes especifically designed for testing. If I would see that, I would wonder what’s the value of MIX_ENV env var 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment