Skip to content

Loading…

Test for slowlog #295

Closed
wants to merge 1 commit into from

3 participants

@nitesh123

Hi, as discussed at the issue#264.

Here is the pull request with a test for slowlog. The new parser seem to work well for slowlog.

For the old parser, i have a patch here gist. Those still on old parser can use this.

Nitesh

@DTrejo DTrejo added a commit that closed this pull request
@nitesh123 nitesh123 test.js: slowlog. Closes #295
Signed-off-by: DTrejo <david.daniel.trejo@gmail.com>
0f5b43a
@DTrejo DTrejo closed this in 0f5b43a
@DTrejo

Thank you!

@DTrejo

Thank you! Strangely, the first time I ran test.js with your test, it failed, and the rest of the times it succeeded.

Going to look into that. Not sure why it's happening: @brycebaril maybe you can reproduce once you pull down the latest code?

@nitesh123

@DTrejo thanks
is the test failing in the subsequent runs?
For me the test run was successful.

Do let me know if this comes again.

@brycebaril
NodeRedis member

I haven't looked yet, but is it possible that it was failing for @DTrejo because of an empty slowlog? I.e. you had just started the Redis server?

@brycebaril
NodeRedis member

My first run with this test also failed, but the subsequent runs succeeded. This should probably be researched and fixed.

In fact, the first test run after a Redis server restart will always cause this test to fail for me.

There are also some formatting issues with this patch that would be great to have cleaned up -- mostly with indentation and spacing consistency.

@nitesh123

@DTrejo @brycebaril . there was a minor glitch in the test code, that caused the test to fail in the first attempt. I had a running server so i was not able repro this issue before. Thanks @brycebaril for pointing out.

Glitch : slowlog reset took less than 10000 ms hence it was not added into slowlog list, in the subsequent test runs the slowlog-log-slower-than param was set to process all commands, hence it considered all commands starting from slowlog reset.

I have fixed the issue and also indentation. with pull request: 381.

Please refer to the latest commit for the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Sep 27, 2012
  1. @nitesh123

    test for slowlog

    nitesh123 committed
Showing with 15 additions and 0 deletions.
  1. +15 −0 test.js
View
15 test.js
@@ -1568,6 +1568,21 @@ tests.ENABLE_OFFLINE_QUEUE_FALSE = function () {
});
};
+tests.SLOWLOG = function () {
+ var name = "SLOWLOG";
+ client.slowlog('reset',require_string("OK", name));
+ client.config('set', 'slowlog-log-slower-than', 0,require_string("OK", name));
+ client.set('foo','bar',require_string("OK", name));
+ client.get('foo',require_string("bar", name));
+ client.slowlog('get',function(err,res){
+ assert.equal(res.length, 4, name);
+ assert.equal(res[0][3].length, 2, name);
+ assert.deepEqual(res[1][3], ['set', 'foo', 'bar'], name);
+ assert.deepEqual(res[3][3], ['slowlog', 'reset'], name);
+ next(name);
+ });
+}
+
// TODO - need a better way to test auth, maybe auto-config a local Redis server or something.
// Yes, this is the real password. Please be nice, thanks.
tests.auth = function () {
Something went wrong with that request. Please try again.