use Test::LWP::Recorder to mock requests

[ironcamel]

This mocks our tests so that a stripe key or network connection will not
be necessary to run the tests. This also gives a major speedup. We can
still run real tests hitting the actual stripe webservice as we do now, if
a stripe api key is provided.

[tildedave]

Omg yay

[danschmidt5189]

This is fantastic. I'm reviewing, should be OK but I just want to familiarize myself with the changes.

[danschmidt5189]

I don't think we need skip_unless_has_secret anymore.

[ironcamel]

We need to keep it because t/07-options.t still needs it. We can't use a mock in the idempotency_key test https://github.com/Crowdtilt/WebService-Stripe/blob/lwp-recorder/t/07-options.t#L17

[danschmidt5189]

Would filter_header => 'Idempotency-Key' solve that? I'm not sure if that affects how LWP Recorder would match a request with a response.

[ironcamel]

No, it wouldn't solve it. The problem is that LWP Recorder has no way to differentiate between 2 identical requests. It would only store the response for one of them. That would make mocking this test useless.

[ironcamel]

The tests run in under 1s now:

➜  WebService-Stripe git:(lwp-recorder) prove -l -I ../Test-LWP-Recorder/lib
t/01-customers.t .... ok   
t/02-charges.t ...... ok   
t/03-banks.t ........ ok   
t/04-transfers.t .... ok   
t/05-headers.t ...... ok   
t/06-balance.t ...... ok   
t/07-options.t ...... skipped: PERL_STRIPE_TEST_API_KEY is required
t/08-file-upload.t .. ok   
t/09-refunds.t ...... ok   
t/10-accounts.t ..... ok   
All tests successful.
Files=10, Tests=26,  1 wallclock secs ( 0.03 usr  0.01 sys +  1.15 cusr  0.06 csys =  1.25 CPU)
Result: PASS

[danschmidt5189]

I really don't like no_-prefixed variables. Affirmative variables (e.g. mock_lwp => 1) tend to be clearer / less confusing. Suggested rewrite:

memoize 'stripe';
sub stripe {
    my %params = @_;
    $params{'api_key'}         //= api_key;
    $params{'version'}         //= '2014-12-17';
    $params{'mock_lwp_record'} //= $ENV{'PERL_STRIPE_MOCK_RECORD'};
    $params{'mock_lwp'}        //= $ENV{'PERL_STRIPE_MOCK'}
        // !$params{'api_key'};

    my %client;
    $client{'api_key'} = $params{'api_key'};
    $client{'version'} = $params{'version'};
    $client{'ua'}      = Test::LWP::Recorder->new({
        record    => $params{'mock_lwp_record'},
        cache_dir => 't/LWPCache/' . basename($0),
    }) if $params{'mock_lwp'};

    state $stripe = WebService::Stripe->new(%client);
    return $stripe;
}

[danschmidt5189]

👍 Had a Q about using filter_header to handle the Idempotency-Key though.

[ironcamel]

We can't merge this until this patch gets accepted riemann42/Test-LWP-Recorder#3 , or the author lets me take over that module, or we aggressively fork it.

[ironcamel]

Another option is to use this module instead https://metacpan.org/pod/Test::VCR::LWP

[danschmidt5189]

That module seems much harder to use suite-wide than the current one.