realestate-com-au / akamai-rspec

Test your akamai configuration with rspec
24 stars 13 forks source link

Check forbidden cacheability #6

Closed patrobinson closed 8 years ago

patrobinson commented 8 years ago

There is a good use case for checking a page is both forbidden and not cacheable. (Steam could do with something like this :P)

We shouldn't be checking the response code inside the not_be_cached matcher, instead we should be chaining matchers together if required. This PR introduces this functionality, so you are able to do:

expect(url).to not_be_cached.and be_forbidden
patrobinson commented 8 years ago

I have a spec failure:

Failures:

  1) be_served_from_origin should fail on 300 and correct origin
     Failure/Error: expect { expect(DOMAIN + '/redirect').to be_served_from_origin('originsite.example.com') }
       expected RuntimeError but nothing was raised
     # ./spec/functional/x_cache_headers_spec.rb:47:in `block (2 levels) in <top (required)>'

Finished in 1.03 seconds (files took 0.48039 seconds to load)
66 examples, 1 failure, 2 pending

But digging into this failure I believe it's not caused by my changes, simple unmasking a failure that was somehow masked. This is probably because your matchers are returning RuntimeErrors rather than false (see #5), but I can't figure out where this issue lies.

Also the x_cache_headers matchers look weird, I'm not sure if it's appropriate to call an expectation within a matcher? I would instead expect all functionality to be split out into methods, as this is both confusing and also possibly a cause of this failure.

patrobinson commented 8 years ago

Ok, I've significantly refactored the x_cache_headers matchers, to make it more readable and now the tests pass :sparkles:

beegibson commented 8 years ago

Thanks for the PR, I'll review it at some point over this weekend.

beegibson commented 8 years ago

Thanks, the x_cache stuff is so much clearer now!