Skip to content

Specify telephony service in specs#1669

Merged
monfresh merged 1 commit intomasterfrom
mb-fix-twilio-service-spec
Sep 11, 2017
Merged

Specify telephony service in specs#1669
monfresh merged 1 commit intomasterfrom
mb-fix-twilio-service-spec

Conversation

@monfresh
Copy link
Contributor

Why: To make sure the correct class is called each time. Otherwise,
a class that was set by a previous spec will still be in effect for the
next test, but might be the wrong class for that test.

**Why**: To make sure the correct class is called each time. Otherwise,
a class that was set by a previous spec will still be in effect for the
next test, but might be the wrong class for that test.
@monfresh
Copy link
Contributor Author

@zachmargolis
Copy link
Contributor

This kind of global mutable state is usually a smell, should we change how this is set (ex make it a constructor param) or add some before-each stuff to the suite to reset to a known value?

Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM thanks for catching!

@monfresh
Copy link
Contributor Author

I'll look into it, but I don't think a reset would have helped here. We have several specs that each depend on a specific service. For testing SMS, we want to use FakeSms, but for voice, we want to use FakeVoiceCall. The issue that caused the test to fail in Circle CI is that the test was expecting the FakeVoiceCall service, but because it was not set in the test, it used whatever was set by the last test, which happened to be FakeSms in this particular seed.

@monfresh monfresh merged commit 5962495 into master Sep 11, 2017
@monfresh monfresh deleted the mb-fix-twilio-service-spec branch September 11, 2017 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants