reply to https://gist.github.com/dfkaye/5959089#comment-859856
Quoted:
"I had a function whose job was to take a URL and a callback and, once complete, invoke the callback passing in the content found at that URL. If there was a request in-progress, the passed URL was added to a queue. Also, once a request was finished the result was cached so subsequent requests to the same URL would be instantaneous.
"In the above case, the public function was pretty simple. Most of the logic was delegated to private functions that processed the queue, did the requesting, maintained the cache, etc. These functions offered no value on their own, so it didn't make sense to expose them publicly. Not to mention the fact that I didn't want the user manually altering the cache.
"Anyway, it got to the point when I realized that simply testing the single public method was definitely not going to be sufficient. I could test that the requestURL() function returned a certain result, but how could I know for sure that the cache was working? How could I know that results were processed in the correct order? I found that I definitely wanted to write tests to make sure that multiple requests to the same URL wouldn't result in seperate AJAX requests, but in order to do that I needed to publicly expose those private functions and the private cache object."
In other words, we call requestURL and either:
a cached response is passed to callback
or
we make a real request
put the request in queue
cache the response
pass the response to callback
+++++++++++
The question then is, how would I have handled this?
Nutshell
command-query separation - Add a public method or two so that the internal methods are callable from elsewhere, and modify the internal methods to accept a status object param that they can pass around. That allows us to pass in mock states and verify that the internal methods make decisions based on it, and then update that state as expected.
Aside - I don't agree that the internal methods should be hidden - but I would keep the request queue and the response cache private, certainly. I might add a clear() method to flush the cache. Maybe.
Query
First, though, the more important thing with respect to testing is that I'd have requestURL() return a status object with the url, request, queue, cache, and response information:
var status = requestURL(url, callback);
This would be a snapshot copy of the current underlying state of things, which the internal functions would have access to right away. Depending on what we need, we could see some of the following fields:
{
url : url
status: 'requested' | 'processing' | 'complete'
cached: 1 if the internal result cache contains url and a result, else 0
queue: 0 if nothing else in queue, else 1, 2,... etc., indicating the order in the queue
done: 1 if really complete, 0 if not
error: maybe maybe not
}
url is the param passed initially, status could be a string indicating where in the method chain the new request has progressed, whether the url is already cached, index in queue, etc. We might even add an error field in case something blows up underneath -- set an error message there and we can examine it later.
Second, I'd make a public query method called status(url) that can return state information at any time. That way, the command method, requestURL(), could call it and make decisions (use cache, make request, queue request, mark complete) - and even pass the status object to the other methods like a collecting parameter, and finally return it as its last execution step - which is important for us as requests are asynchronous. I imagine the sequence is something like the following:
function requestURL(url, callback) {
var collector = status(url);
var cache = collector.cache;
if empty cache {
queueRequest(collector, function(response) {
cache[url] = response;
callback(response)
})
} else {
callback(cache)
}
return collector;
}
Of course, the internal command method queueRequest() gets more complicated as it checks a the map of requests, the queue to find its index, its completion state or an error state - all that is implementation detail - but the possibility of re-using status(url) and/or passing the status collector around, means you can query the status for a given url at ANY time.
Command
Third, there's still something wrong with the picture - there's another internal function that's doing real requests against a live server. That needs to be controlled, toggled, mocked or replaced. I'd pull that method up somehow, or maybe create a public requestfactory method that receives the status object for a url, then calls the internal real request method, etc.
function requestFactory(collector, callback?) {
realRequest(url, callback)
}
You could then use the Function() body replace trick I described here => https://gist.github.com/dfkaye/5987716 - "Mocking - not testing - private functions in JavaScript" - to replace the "realRequest" function name in the public requestFactory.
IN CLOSING
As I said above, I don't agree that any of the hidden methods needs to be hidden, or that the user should be protected from clearing the response cache - that may well be helpful for users to do, in fact - and convenient for you as you exercise the code under test.
Mark Ethan Trostler writes in Testable JavaScript, p. 25, "Exactly what battle are you fighting to keep your code more complex?"
But if you must keep things private, then enabling more query-command separation and passing state information around is probably the most testable way.