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

Fix unstable behavior without async: false #126

Merged
merged 1 commit into from
Jan 28, 2018

Conversation

BKStephens
Copy link
Contributor

Since Meck mocks globally, I added a process/module (MockLock) that
controls access to this resource. Now, before ExVCR.Mock.mock_methods calls
:meck.expect, it request a lock from MockLock. When the lock becomes available,
MockLock sends :lock_granted to the calling process. When the calling process is
done, it calls ExVCR.MockLock.release_lock(). Right before MockLock sends
:lock_granted to calling process, it starts to monitor the process. If the
calling process goes down, MockLock will be notified and release the lock.

I also had to make ExVCR.Setting work with async. In order to achieve
this, I used the same pattern for naming the ets table as in
ExVCR.Adapter.Hackney.Store, which is to include the pid in the ets
table name. This does introduce a breaking change: calling ExVCR.Config
functions outside of test process, such as in setup_all, will not work.
Instead, ExVCR.Config should be called in setup or test. I have
documented this in the README Notes section.

Another change is that I removed the clear_mock option. Now,
:meck.unload is called for each test every time. This is necessary for
MockLock/async to work correctly.

Since Meck mocks globally, I added a process/module (MockLock) that
controls access to this resource. Now, before ExVCR.Mock.mock_methods calls
:meck.expect, it request a lock from MockLock. When the lock becomes available,
MockLock sends :lock_granted to the calling process. When the calling process is
done, it calls ExVCR.MockLock.release_lock(). Right before MockLock sends
:lock_granted to calling process, it starts to monitor the process. If the
calling process goes down, MockLock will be notified and release the lock.

I also had to make ExVCR.Setting work with async. In order to achieve
this, I used the same pattern for naming the ets table as in
ExVCR.Adapter.Hackney.Store, which is to include the pid in the ets
table name. This does introduce a breaking change: calling ExVCR.Config
functions outside of test process, such as in setup_all, will not work.
Instead, ExVCR.Config should be called in setup or test. I have
documented this in the README Notes section.

Another change is that I removed the clear_mock option. Now,
:meck.unload is called for each test every time. This is necessary for
MockLock/async to work correctly.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.4%) to 93.684% when pulling 489826b on BKStephens:fix_unstable_async into 4fdd8b0 on parroty:master.

1 similar comment
@coveralls
Copy link

coveralls commented Jan 20, 2018

Coverage Status

Coverage increased (+0.4%) to 93.684% when pulling 489826b on BKStephens:fix_unstable_async into 4fdd8b0 on parroty:master.

@bgeihsgt
Copy link

@parroty have you had a chance to look at this? I'm also interested in having this for my team.

@parroty parroty merged commit 04c4e36 into parroty:master Jan 28, 2018
@parroty
Copy link
Owner

parroty commented Jan 28, 2018

Thanks for the extensive implementation! I'll be pushing.

@andrewtimberlake
Copy link

Is it possible this broke how ExVCR.Config.cassette_library_dir/1 works? After upgrading all my tests just stopped working and I had new cassettes created in the default path.
I reverted to 0.9.1 and all my tests work again.

@BKStephens
Copy link
Contributor Author

@andrewtimberlake This change did bring about a breaking change that calling ExVCR.Config functions from outside the test process will not work (such as within setup_all).

You should still be able to call ExVCR.Config.cassette_library_dir/1 from a setup block or configure it in config/test.exs. Are you calling calling ExVCR.Config.cassette_library_dir/1 from within a setup_all? If so, try moving it to setup.

@nihalgonsalves
Copy link
Contributor

@parroty I don't think that this was a suitable change for a minor version bump. The readme prior to this PR had multiple examples that used setup_all. Deprecating that, like @BKStephens noted originally in this thread, is a breaking change.

0.10.0 also seems to have caused some timeout issues that I haven't had time to investigate, so I have been forced to downgrade manually to 0.9.1 for now as well.

@andrewtimberlake
Copy link

I did have the config in setup_all as @nihalgonsalves mentioned that was in the docs. When I get a chance I’ll move to setup and update again. Thanks

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

Successfully merging this pull request may close these issues.

6 participants