On Wed, Aug 27, 2008 at 3:42 PM, John Hjelmstad <[EMAIL PROTECTED]> wrote:
> On Wed, Aug 27, 2008 at 2:43 PM, Kevin Brown <[EMAIL PROTECTED]> wrote: > > > On Tue, Aug 26, 2008 at 4:07 PM, John Hjelmstad <[EMAIL PROTECTED]> > wrote: > > > > > On Tue, Aug 26, 2008 at 3:48 PM, Kevin Brown <[EMAIL PROTECTED]> wrote: > > > > > > > I don't see any reason to change that first test (though, honestly, > I'd > > > > stop > > > > using the mock entirely for HttpRequest here. I'm not really sure why > > > > there's a mock being used in the first place). > > > > > > > > > I changed the first test from assertSame to assertEquals because > > > addResponse > > > now makes a new HttpResponse object when caching it, so it's not > actually > > > equivalent. > > > > > > I was referring to changing setExpirationTime to setCacheTtl, not > changing > > assertSame to assertEquals. > > > Aha, that makes sense. I changed to setCacheTtl due to the fact that > AbstractHttpCache now sets Cache-Control when request.getCacheTtl() != -1. > As such, I wanted the response object I'm caching to have the same header > so > they can be compared using .equals(). > > Basically I wanted this to test just the rewriter ops in a scenario in > which > caching is enabled (rather than testing timeout behavior). > > Does that seem reasonable, at least for fixing the current bug? Clearly we > need to clean up this logic in the future anyway. > ok.
