> > -    EasyMock.expect(req.getToken()).andReturn(FAKE_GADGET_TOKEN);
> > +
> >  
> > EasyMock.expect(req.getAttribute(EasyMock.isA(String.class))).andReturn(FAKE_GADGET_TOKEN);
>
>
> instead of isA(String.class) can't you just do request.getAttribute("st")?
> it would make the test more concrete.

This seemed the lesser of evils. The attribute value is defined in the
enum AuthInfo.Attribute and evaluates to
"org.apache.shindig.social.core.oauth.AuthInfo.SECURITY_TOKEN", and
the choices were:
1. Use isA()... fragile based on order of attribute evaluation (what I did).
2. Hard code the string
"org.apache.shindig.social.core.oauth.AuthInfo.SECURITY_TOKEN". This
is fragile based on the AuthInfo class name and uses an internal
implementation detail
3. Make the enum public. This also exposed a detail that we didn't
want generally available - gets/sets should go through the public API.
Do we have an annotation for functions / objects that are only
supposed to be used for testing?
4. Use "st" as the attribute value. Generally speaking it's good
practice to uniquely prefix servlet attributes to avoid collisions.
Also, this might encourage not going through the API to get/set the
token.

Given these details, if one of the other options still makes sense I'm
happy to switch.

Evan

Reply via email to