parroty / exvcr

HTTP request/response recording library for elixir, inspired by VCR.
MIT License
712 stars 133 forks source link

disable global mock #159

Open ayrat555 opened 3 years ago

ayrat555 commented 3 years ago

I just updated my deps to the latest version, some tests started failing because instead of exvcr they use fake_server. is it possible to disable global mock?

parroty commented 3 years ago

Thanks for the report. Is there any way to access the code (or sample code) which I can reproduce the error?

ayrat555 commented 3 years ago

I don't have an open example but I think the issue can be easily reproduced by mocking some tests with exvcr but allowing other tests to make requests without mocks.

Here's stacktrace I get:

         ** (EXIT) an exception was raised:
             ** (ArgumentError) argument error
                 :erlang.apply([], :options, [])
                 (exvcr 0.12.0) lib/exvcr/recorder.ex:71: ExVCR.Recorder.options/1
                 (exvcr 0.12.0) lib/exvcr/handler.ex:178: ExVCR.Handler.ignore_request?/2
                 (exvcr 0.12.0) lib/exvcr/handler.ex:17: ExVCR.Handler.get_response/2
                 (httpoison 1.7.0) lib/httpoison/base.ex:796: HTTPoison.Base.request/6
surik commented 3 years ago

@ayrat555 thank you for the stack trace. I think I faced the same issue when I'd been working on the global mock. So I added the test for such case and was sure that this fixed.

For a better understanding of the issue, could you check if it is the same when you run mix test --max-cases 1?

ayrat555 commented 3 years ago

What is the reason for global mock? What problem does it solve? Can it be optional or turned off by default?

parroty commented 3 years ago

Thank you for the information and analysis. My understanding is that it improve the performance of the tests. But, I think I was optimistic about the compatibility part, and wondering whether to revert the change and re-introduce as optional feature (not confident yet, though..).

surik commented 3 years ago

@ayrat555

What is the reason for global mock? What problem does it solve?

The global mock is an attempt to address a general issue with exvcr being slow, see https://github.com/parroty/exvcr/issues/107

In general, every use_cassette took around 500 ms so if you extensively use cassette it could spend minutes doing :meck.expect/2 and :meck.unload/1.

Can it be optional or turned off by default?

Now it doesn't have such a flag but for sure it can be added. I understand that this maybe is critical for your application so I can suggest you switch back to 0.11.2 which doesn't have the global mock yet.

I still believe that the issue can be fixed with the global mock. I'm currently trying to debug it but unfortunately no luck to get the same case. More information would help. For example, if you would run your tests with mix test --max-cases 1 and check if it is the same, or help finding minimum test case to reproduce it.

@parroty

I don't want to lose an ability to speed up my tests so I can take a look at the flag that could enable the global mock as an experimental feature.

parroty commented 3 years ago

Thank you for the detailed description.

I don't want to lose an ability to speed up my tests so I can take a look at the flag that could enable the global mock as an experimental feature.

Thank you for looking into this, I appreciate a lot.

surik commented 3 years ago

There is a PR to make global mock experimental and disabled by default https://github.com/parroty/exvcr/pull/160

kpanic commented 3 years ago

I have a similar error reported in https://github.com/parroty/exvcr/issues/159#issuecomment-705386461

      :erlang.apply([], :options, [])
       (exvcr 0.12.1) lib/exvcr/handler.ex:30: ExVCR.Handler.get_response_from_cache/2
       (exvcr 0.12.1) lib/exvcr/handler.ex:20: ExVCR.Handler.get_response/2
       (httpoison 1.7.0) lib/httpoison/base.ex:796: HTTPoison.Base.request/6
       (goth 1.2.0) lib/goth/client.ex:83: Goth.Client.get_access_token/3
       (goth 1.2.0) lib/goth/token.ex:151: Goth.Token.retrieve_and_store!/2

from commit d757ad434aa877deeb3868bc2efd4e7bb372cb93

In addition to this, when I don't use the global_mock but use_cassette looks like Goth.Client.get_access_token/3 is not recorded. I am calling it from my application.

After recording the cassette I get

** (ExVCR.RequestNotMatchError) Request did not match with any one in the current cassette: 
fixture/vcr_cassettes/fixture_30ca562f-c13c-40c7-8825-e22ce32fb692.json.
     Delete the current cassette with [mix vcr.delete] and re-record.

     Request: [:post, "url", 
         [{"Content-Type", "application/x-www-form-urlencoded"}], 
         {:form, [grant_type: "urn:ietf:params:oauth:grant-type:jwt-bearer", 
         assertion: "somepayload"], []]

Any hints about this latter problem?

surik commented 3 years ago

@kpanic there is a similar report about not recording cassettes with HTTPoison #162. I will take a look at both issues. Is there any chance that you can post a minimum test case that helps to debug it?

kpanic commented 3 years ago

@surik I'll try to see if I can re-produce the issue in a fresh repo. Tomorrow or the next week. Thanks for the quick reply!

kpanic commented 3 years ago

@surik here you go https://github.com/kpanic/global_mock

surik commented 3 years ago

@kpanic thank you for the test case. I think I found the issue with :erlang.apply([], :options, []).

When we have global_mock enabled a process starts here, that by default it has [] as a state. So if I would run the application without using cassettes I get:

iex(1)>  ExVCR.Actor.CurrentRecorder.get()
[]

But for bypassing cassettes as needed in your test case we expect a nil in these places:

I will fix it and also try to add a test case.

The problem with not recording cassettes is not clear for me yet. I'm not sure if related to the above issue or it is something different.

kpanic commented 3 years ago

No worries about the latter problem. I might create a separate issue for that one Thanks a lot!