All tests pass, moved appenders into separate files, so that extra ones can be added easily

lib/log4js.js

@@ -18,8 +18,6 @@
  * @fileoverview log4js is a library to log in JavaScript in similar manner
  * than in log4j for Java. The API should be nearly the same.
  *
- * This file contains all log4js code and is the only file required for logging.
- *
  * <h3>Example:</h3>
  * <pre>
  *  var logging = require('log4js');
@@ -37,6 +35,8 @@
  *  log.trace("trace me" );
  * </pre>
  *
+ * NOTE: the authors below are the original browser-based log4js authors
+ * don't try to contact them about bugs in this version :)
  * @version 1.0
  * @author Stephan Strittmatter - http://jroller.com/page/stritti
  * @author Seth Chisamore - http://www.chisamore.com
@@ -136,7 +136,9 @@ function configureAppenders(appenderList) {
     clearAppenders();
     if (appenderList) {
 	appenderList.forEach(function(appenderConfig) {
-	    var appender = appenderMakers[appenderConfig.type](appenderConfig);
+            var appender;
+            appenderConfig.makers = appenderMakers;
+	    appender = appenderMakers[appenderConfig.type](appenderConfig);
 	    if (appender) {
 		addAppender(appender, appenderConfig.category);
 	    } else {
@@ -144,7 +146,7 @@ function configureAppenders(appenderList) {
 	    }
 	});
     } else {
-	addAppender(consoleAppender);
+	addAppender(consoleAppender());
     }
 }
 
@@ -247,7 +249,6 @@ function configure (configurationFileOrObject) {
     }
     if (config) {
 	try {
-            config.makers = appenderMakers;
 	    configureAppenders(config.appenders);
 	    configureLevels(config.levels);
         } catch (e) {
@@ -283,10 +284,11 @@ function replaceConsole(logger) {
     }
 
     ['log','debug','info','warn','error'].forEach(function (item) {
+        if (console['_preLog4js_log'] === undefined) {
             console['_preLog4js_'+item] = console[item];
+        }
         console[item] = replaceWith(item === 'log' ? logger.info : logger[item]);
     });
-
 }
 
 function loadAppenders() {

log-rolling.js

@@ -0,0 +1,28 @@
+var log4js = require('./lib/log4js')
+, log
+, i = 0;
+log4js.configure({
+  "appenders": [
+      {
+          "type": "file",
+          "filename": "tmp-test.log",
+          "maxLogSize": 1024,
+          "backups": 3,
+          "pollInterval": 0.1,
+          "category": "test"
+      },
+      {
+          type: "console"
+        , category: "console"
+      }
+  ]
+});
+log = log4js.getLogger("test");
+
+function doTheLogging(x) {
+    log.info("Logging something %d", x);
+}
+
+for ( ; i < 100000; i++) {
+    doTheLogging(i);
+}

test/fileAppender.js

@@ -14,6 +14,7 @@ function remove(filename) {
 }
 
 vows.describe('log4js fileAppender').addBatch({
+
     'with default fileAppender settings': {
         topic: function() {
             var testFile = __dirname + '/fa-default-test.log'
@@ -122,5 +123,61 @@ vows.describe('log4js fileAppender').addBatch({
             }
         }
     }
+}).addBatch({
+    'configure' : {
+        'with fileAppender': {
+	    topic: function() {
+	        var log4js = require('../lib/log4js')
+              , logger;
+	        //this config file defines one file appender (to ./tmp-tests.log)
+	        //and sets the log level for "tests" to WARN
+                log4js.configure('test/log4js.json');
+                logger = log4js.getLogger('tests');
+	        logger.info('this should not be written to the file');
+	        logger.warn('this should be written to the file');
+
+                fs.readFile('tmp-tests.log', 'utf8', this.callback);
+	    },
+	    'should load appender configuration from a json file': function(err, contents) {
+	        assert.include(contents, 'this should be written to the file\n');
+                assert.equal(contents.indexOf('this should not be written to the file'), -1);
+	    },
+            'and log rolling': {
+                topic: function() {
+                    var sandbox = require('sandboxed-module')
+                  , that = this
+                  , fileAppender = sandbox.require(
+                      '../lib/appenders/file.js'
+                    , { requires:
+                        { 'fs':
+                          {
+                              watchFile: function(filename, options, cb) {
+                                  that.callback(null, filename);
+                              }
+                            , createWriteStream: function() {
+                                  return {
+                                      on: function() {}
+                                    , end: function() {}
+                                    , destroy: function() {}
+                                  };
+                              }
+                          }
+                        }
+                      }
+                    );
+
+                    fileAppender.configure({
+                        filename: "cheese.log"
+                      , maxLogSize: 100
+                      , backups: 5
+                      , pollInterval: 30
+                    });
+                },
+                'should watch the log file': function(watchedFile) {
+                    assert.equal(watchedFile, 'cheese.log');
+                }
+            }
+        }
+    }
 
 }).export(module);

test/layouts.js

@@ -102,29 +102,24 @@ vows.describe('log4js layouts').addBatch({
             error = new Error("Some made-up error"),
             stack = error.stack.split(/\n/);
 
-            event.data = ['this is a test', error];
+            event.data = ['this is a test ', error];
             output = layout(event);
             lines = output.split(/\n/);
 
-            assert.length(lines, stack.length+1);
+            assert.length(lines, stack.length);
-            assert.equal(lines[0], "[2010-12-05 14:18:30.045] [DEBUG] tests - this is a test");
+            assert.equal(lines[0], "[2010-12-05 14:18:30.045] [DEBUG] tests - this is a test Error: Some made-up error");
-            assert.equal(lines[1], "Error: Some made-up error");
             for (var i = 1; i < stack.length; i++) {
-                assert.equal(lines[i+1], stack[i]);
+                assert.equal(lines[i+1], stack[i+1]);
             }
         },
         'should output any extra data in the log event as util.inspect strings': function(args) {
             var layout = args[0], event = args[1], output, lines;
-            event.data = ['this is a test', {
+            event.data = ['this is a test ', {
                     name: 'Cheese',
                     message: 'Gorgonzola smells.'
             }];
             output = layout(event);
-            lines = output.split(/\n/);
+            assert.equal(output, "[2010-12-05 14:18:30.045] [DEBUG] tests - this is a test { name: 'Cheese', message: 'Gorgonzola smells.' }");
-
-            assert.length(lines, 2);
-            assert.equal(lines[0], "[2010-12-05 14:18:30.045] [DEBUG] tests - this is a test");
-            assert.equal(lines[1], "{ name: 'Cheese', message: 'Gorgonzola smells.' }");
         }
     },
 

test/logLevelFilter.js

@@ -0,0 +1,69 @@
+var vows = require('vows')
+, fs = require('fs')
+, assert = require('assert');
+
+function remove(filename) {
+    try {
+        fs.unlinkSync(filename);
+    } catch (e) {
+        //doesn't really matter if it failed
+    }
+}
+
+vows.describe('log4js logLevelFilter').addBatch({
+    'appender': {
+        topic: function() {
+            var log4js = require('../lib/log4js'), logEvents = [], logger;
+            log4js.clearAppenders();
+            log4js.addAppender(log4js.logLevelFilter('ERROR', function(evt) { logEvents.push(evt); }), "logLevelTest");
+            logger = log4js.getLogger("logLevelTest");
+            logger.debug('this should not trigger an event');
+            logger.warn('neither should this');
+            logger.error('this should, though');
+            logger.fatal('so should this');
+            return logEvents;
+        },
+        'should only pass log events greater than or equal to its own level' : function(logEvents) {
+            assert.length(logEvents, 2);
+            assert.equal(logEvents[0].data[0], 'this should, though');
+            assert.equal(logEvents[1].data[0], 'so should this');
+        }
+    },
+
+    'configure': {
+        topic: function() {
+            var log4js = require('../lib/log4js')
+          , logger;
+
+            remove(__dirname + '/logLevelFilter.log');
+            remove(__dirname + '/logLevelFilter-warnings.log');
+
+            log4js.configure('test/with-logLevelFilter.json');
+	    logger = log4js.getLogger("tests");
+	    logger.info('main');
+	    logger.error('both');
+	    logger.warn('both');
+	    logger.debug('main');
+
+	    return logger;
+        },
+	'tmp-tests.log': {
+	    topic: function() {
+		fs.readFile(__dirname + '/logLevelFilter.log', 'utf8', this.callback);
+            },
+            'should contain all log messages': function(contents) {
+                var messages = contents.trim().split('\n');
+	        assert.deepEqual(messages, ['main','both','both','main']);
+            }
+        },
+        'tmp-tests-warnings.log': {
+	    topic: function() {
+		fs.readFile(__dirname + '/logLevelFilter-warnings.log','utf8',this.callback);
+            },
+            'should contain only error and warning log messages': function(contents) {
+                var messages = contents.trim().split('\n');
+	        assert.deepEqual(messages, ['both','both']);
+	    }
+        }
+    }
+}).export(module);

test/logging.js

@@ -55,81 +55,33 @@ vows.describe('log4js').addBatch({
 
     },
 
-    'configure' : {
+    'invalid configuration': {
-	topic: function() {
+        'should throw an exception': function() {
-	    var messages = {}, fakeFS = {
+            assert.throws(function() {
-		createWriteStream: function(file) {
+                log4js.configure({ "type": "invalid" });
-		    return {
+            });
-                        write: function(message) {
-                            if (!messages.hasOwnProperty(file)) {
-			        messages[file] = [];
-		            }
-		            messages[file].push(message);
-                        }
-                      , end: function() {}
-                      , destroySoon: function() {}
-                    };
-		},
-                readdirSync: function(dir) {
-                    return require('fs').readdirSync(dir);
-                },
-		readFileSync: function(file, encoding) {
-		    return require('fs').readFileSync(file, encoding);
-		},
-                watchFile: function(file) {
-                    messages.watchedFile = file;
         }
     },
-	    log4js = sandbox.require(
-                '../lib/log4js'
-              , {
-                  requires: {
-                      'fs': fakeFS
-                  }
-              }
-            );
-	    return [ log4js, messages ];
-	},
-	'should load appender configuration from a json file': function(args) {
-	    var log4js = args[0], messages = args[1];
-	    delete messages['tmp-tests.log'];
-	    log4js.clearAppenders();
-	    //this config file defines one file appender (to ./tmp-tests.log)
-	    //and sets the log level for "tests" to WARN
-	    log4js.configure('test/log4js.json');
-	    var logger = log4js.getLogger("tests");
-	    logger.info('this should not be written to the file');
-	    logger.warn('this should be written to the file');
-	    assert.length(messages['tmp-tests.log'], 1);
-	    assert.equal(messages['tmp-tests.log'][0], 'this should be written to the file\n');
-	},
-	'should handle logLevelFilter configuration': function(args) {
-	    var log4js = args[0], messages = args[1];
-	    delete messages['tmp-tests.log'];
-	    delete messages['tmp-tests-warnings.log'];
-	    log4js.clearAppenders();
-	    log4js.configure('test/with-logLevelFilter.json');
-	    var logger = log4js.getLogger("tests");
-	    logger.info('main');
-	    logger.error('both');
-	    logger.warn('both');
-	    logger.debug('main');
 
-	    assert.length(messages['tmp-tests.log'], 4);
+    'configuration when passed as object': {
-	    assert.length(messages['tmp-tests-warnings.log'], 2);
+        topic: function() {
-	    assert.deepEqual(messages['tmp-tests.log'], ['main\n','both\n','both\n','main\n']);
+	    var appenderConfig
-	    assert.deepEqual(messages['tmp-tests-warnings.log'], ['both\n','both\n']);
+          , log4js = sandbox.require(
-	},
+              '../lib/log4js'
-        'should handle fileAppender with log rolling' : function(args) {
+            , { requires:
-            var log4js = args[0], messages = args[1];
+                { './appenders/file.js':
-            delete messages['tmp-test.log'];
+                  {
-            log4js.configure('test/with-log-rolling.json');
+                      name: "file"
-            assert.equal(messages.watchedFile, 'tmp-test.log');
+                    , appender: function() {}
-        },
+                    , configure: function(configuration) {
-        'should handle an object or a file name': function(args) {
+                          appenderConfig = configuration;
-	    var log4js = args[0],
+                          return function() {};
-	    messages = args[1],
+                      }
-	    config = {
+                  }
+                }
+              }
+          )
+          , config = {
 		"appenders": [
 		    {
 			"type" : "file",
@@ -140,34 +92,88 @@ vows.describe('log4js').addBatch({
 		    }
 		]
 	    };
-	    delete messages['cheesy-wotsits.log'];
 	    log4js.configure(config);
-	    assert.equal(messages.watchedFile, 'cheesy-wotsits.log');
+            return appenderConfig;
+        },
+        'should be passed to appender config': function(configuration) {
+	    assert.equal(configuration.filename, 'cheesy-wotsits.log');
+	}
+    },
+
+    'configuration when passed as filename': {
+        topic: function() {
+            var appenderConfig
+          , configFilename
+          , log4js = sandbox.require(
+                '../lib/log4js'
+              , { requires:
+                  { 'fs':
+                    {
+                        readFileSync: function(filename) {
+                            configFilename = filename;
+                            return JSON.stringify({
+                                appenders: [
+                                    { type: "file"
+                                    , filename: "whatever.log"
+                                    }
+                                ]
+                            });
+                        },
+                        readdirSync: function() {
+                            return ['file.js'];
+                        }
+                    }
+                , './appenders/file.js':
+                    {
+                      name: "file"
+                    , appender: function() {}
+                    , configure: function(configuration) {
+                          appenderConfig = configuration;
+                          return function() {};
+                      }
+                    }
+                  }
+                }
+          );
+            log4js.configure("/path/to/cheese.json");
+            return [ configFilename, appenderConfig ];
+        },
+        'should read the config from a file': function(args) {
+            assert.equal(args[0], '/path/to/cheese.json');
+        },
+        'should pass config to appender': function(args) {
+            assert.equal(args[1].filename, "whatever.log");
         }
     },
 
     'with no appenders defined' : {
         topic: function() {
             var logger
-          , message
+          , that = this
+          , fakeConsoleAppender = {
+                name: "console"
+              , appender: function() {
+                    return function(evt) {
+                        that.callback(null, evt);
+                    }
+                }
+              , configure: function() {
+                    return fakeConsoleAppender.appender();
+                }
+          }
           , log4js = sandbox.require(
               '../lib/log4js'
             , {
-                globals: {
+                requires: {
-                  console: {
+                    './appenders/console.js': fakeConsoleAppender
-                      log: function(msg) {
-                          message = msg;
-                      }
-                  }
                 }
               }
           );
 	    logger = log4js.getLogger("some-logger");
             logger.debug("This is a test");
-            return message;
         },
-        'should default to the console appender': function(message) {
+        'should default to the console appender': function(evt) {
-            assert.isTrue(/This is a test$/.test(message));
+            assert.equal(evt.data[0], "This is a test");
         }
     },
 
@@ -259,7 +265,7 @@ vows.describe('log4js').addBatch({
     'default setup': {
         topic: function() {
             var pathsChecked = [],
-            message,
+            appenderEvent,
             logger,
             modulePath = require('path').normalize(__dirname + '/../lib/log4js.json'),
             fakeFS = {
@@ -281,28 +287,27 @@ vows.describe('log4js').addBatch({
                 }
             },
             fakeConsole = {
-                log : function (msg) { message = msg; },
+                'name': 'console'
-                info: this.log,
+              , 'appender': function () {
-                warn: this.log,
+                    return function(evt) { appenderEvent = evt; }
-                debug: this.log,
+                }
-                error: this.log
+              , 'configure': function (config) {
+                    return fakeConsole.appender();
+                }
             },
 	    log4js = sandbox.require(
                 '../lib/log4js',
                 {
                     requires: {
                         'fs': fakeFS
-                    },
+                      , './appenders/console.js': fakeConsole
-                    globals: {
-                        'console': fakeConsole
                     }
                 }
             );
 
 	    logger = log4js.getLogger('a-test');
             logger.debug("this is a test");
-
+            return [ pathsChecked, appenderEvent, modulePath ];
-            return [ pathsChecked, message, modulePath ];
         },
 
         'should check current directory, require paths, and finally the module dir for log4js.json': function(args) {
@@ -317,27 +322,8 @@ vows.describe('log4js').addBatch({
         },
 
         'should configure log4js from first log4js.json found': function(args) {
-            var message = args[1];
+            var appenderEvent = args[1];
-            assert.equal(message, 'this is a test');
+            assert.equal(appenderEvent.data[0], 'this is a test');
-        }
-    },
-
-    'logLevelFilter': {
-        topic: function() {
-            var log4js = require('../lib/log4js'), logEvents = [], logger;
-            log4js.clearAppenders();
-            log4js.addAppender(log4js.logLevelFilter('ERROR', function(evt) { logEvents.push(evt); }), "logLevelTest");
-            logger = log4js.getLogger("logLevelTest");
-            logger.debug('this should not trigger an event');
-            logger.warn('neither should this');
-            logger.error('this should, though');
-            logger.fatal('so should this');
-            return logEvents;
-        },
-        'should only pass log events greater than or equal to its own level' : function(logEvents) {
-            assert.length(logEvents, 2);
-            assert.equal(logEvents[0].data[0], 'this should, though');
-            assert.equal(logEvents[1].data[0], 'so should this');
         }
     },
 

test/with-logLevelFilter.json

@@ -6,7 +6,7 @@
       "level": "WARN",
       "appender": {
         "type": "file",
-        "filename": "tmp-tests-warnings.log", 
+        "filename": "test/logLevelFilter-warnings.log", 
         "layout": { 
           "type": "messagePassThrough" 
         } 
@@ -15,7 +15,7 @@
     { 
       "category": "tests", 
       "type": "file",
-      "filename": "tmp-tests.log", 
+      "filename": "test/logLevelFilter.log", 
       "layout": { 
         "type": "messagePassThrough" 
       }