Exposes raw memcache response for empty values #65

Merged
merged 1 commit into from Oct 20, 2012

Projects

None yet

2 participants

@jkrems
Contributor
jkrems commented Oct 20, 2012

Added test case and fix for setting and getting empty strings

Since I wasn't completely sure if some other parts of the code may be relying on the token-line being passed instead of the data-section whenever the data-section was empty/not present, I tried to keep the fix as local as possible.

This pull request also includes the possibility to run the test suite using npm test and to run the test suite with a custom memcached-server via MEMCACHED__HOST=localhost npm test. Including the "require('should')"-line was necessary for me to get the tests running. Otherwise ok.should was always null.

If you only want to have the actual fix, ignore everything but the four lines in lib/memcached.js.

This pull request refers to: #64

@jkrems jkrems [64] Exposes raw memcache response for empty values
Added test case and fix for setting and getting empty strings
5677ee8
@3rd-Eden
Owner

@jkrems Looks awesome, I'll review this shortly. As for the npm test fixes, I have also been looking in to it as I want to get the test suite running on travis CI

@jkrems
Contributor
jkrems commented Oct 20, 2012

Just out of curiosity: Will you then just configure the multi-server tests to use 3 times the same memcached instance?

(I don't know why I was surprised to learn that one line in your .travis.yml is enough to have a ready-to-use memcached there. Those guys truly are amazing. :D)

@3rd-Eden
Owner

@jkrems the services: memcache will only start a regular instance on localhost + default port. To test multiple servers I probably have to do some silly before_script apt-get install memcache and then run memcache on 3 different ports.

@3rd-Eden 3rd-Eden merged commit 6070ef4 into 3rd-Eden:master Oct 20, 2012
@3rd-Eden 3rd-Eden added a commit that referenced this pull request Oct 20, 2012
@3rd-Eden [minor] minor adjustments to pull request #65 and started on travis-c…
…i integration
6a569e9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment