[ 
https://issues.apache.org/jira/browse/SOLR-4406?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14119803#comment-14119803
 ] 

Steve Davids commented on SOLR-4406:
------------------------------------

bq. So i took a stab at refactoring it to have test methods that more directly 
modeled the list of situations you identified

Personally I prefer having small, self describing test method names instead of 
having 3 methods that do everything and making you really dig in there if any 
one of the tests actually fail. That's why I went the route of building 3 test 
methods per case I described above:

# If a content stream is provided send that back in the writer & output stream
#* testGetContentType
#* testWriteContentStreamViaWriter
#* testWriteContentStreamViaOutputStream
# If no content stream is provided and no base writer is specified verify the 
response is serialized with the default writer (XML)
#* testDefaultBaseWriterGetContentType
#* testDefaultBaseWriterViaWriter
#* testDefaultBaseWriterViaOutputStream
# If no content stream is provided and a base writer is specified serialize 
with the specified writer
#* testJsonBaseWriterGetContentType
#* testJsonBaseWriterViaWriter
#* testJsonBaseWriterViaOutputStream

Personally I think this is one of those "beauty is in the eye of the beholder", 
I kind of prefer the original test but cleanliness and clarity can sometimes be 
subjective (though "initRawResponseWriter" was a poor naming choice, perhaps 
"setBaseWriter" would have been better). 

You are testing a couple more cases that I wasn't looking for before, which is 
always a good thing. All the other changes look good, I'm not hung up on any of 
the test changes.

> RawResponseWriter - doesn't use the configured "base" writer.
> -------------------------------------------------------------
>
>                 Key: SOLR-4406
>                 URL: https://issues.apache.org/jira/browse/SOLR-4406
>             Project: Solr
>          Issue Type: Bug
>          Components: Response Writers
>    Affects Versions: 4.0
>            Reporter: Steve Davids
>            Assignee: Hoss Man
>         Attachments: SOLR-4406.patch, SOLR-4406.patch, SOLR-4406.patch, 
> SOLR-4406.patch, SOLR-4406.patch, SOLR-4406.patch
>
>
> The RawResponseWriter accepts a configuration value for a "base" 
> ResponseWriter if no ContentStream can be detected. The line of code is 
> commented out that would allow this secondary response writer to work. It 
> would be great to uncomment the line and provide an OutputStreamWriter as the 
> writer argument.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org

Reply via email to