From dd6dc1505b68a865b1c37197c133acbf5a5e58e0 Mon Sep 17 00:00:00 2001 From: Brian Hann Date: Mon, 23 Feb 2015 09:41:06 -0600 Subject: [PATCH] fix(Grid): fix buildColumns handling same field When buildColumns received the same field multiple times in columnDefs it would eventually throw an exception due to a typo where it was trying to generate incremental display names. This change fixes that and includes tests for the exception as well as incremental displayName creation. Fixes #2789 --- src/js/core/factories/Grid.js | 4 +- test/karma.debug.conf.js | 144 ++++++++++++++++++++++++++ test/unit/core/factories/Grid.spec.js | 32 ++++++ 3 files changed, 178 insertions(+), 2 deletions(-) create mode 100644 test/karma.debug.conf.js diff --git a/src/js/core/factories/Grid.js b/src/js/core/factories/Grid.js index a13bbbdfc7..a9175cd99e 100644 --- a/src/js/core/factories/Grid.js +++ b/src/js/core/factories/Grid.js @@ -793,8 +793,8 @@ angular.module('ui.grid') return 0; } else { - var numA = a.match(nameRE)[1]; - var numB = b.match(nameRE)[1]; + var numA = a.displayName.match(nameRE)[1]; + var numB = b.displayName.match(nameRE)[1]; return parseInt(numA, 10) > parseInt(numB, 10) ? 1 : -1; } diff --git a/test/karma.debug.conf.js b/test/karma.debug.conf.js new file mode 100644 index 0000000000..c25bbf8ba2 --- /dev/null +++ b/test/karma.debug.conf.js @@ -0,0 +1,144 @@ +// Karma configuration +// Generated on Fri Nov 08 2013 09:25:16 GMT-0600 (Central Standard Time) +var util = require('../lib/grunt/utils.js'); +var grunt = require('grunt'); + +module.exports = function(config) { + config.set({ + + // base path, that will be used to resolve files and exclude + basePath: '../', + + // frameworks to use + frameworks: ['jasmine'], + + // list of files / patterns to load in the browser (we add more dynamically in our tasks) + files: [ + 'bower_components/jquery/jquery.min.js', + 'lib/test/jquery.simulate.js', + 'bower_components/lodash/dist/lodash.min.js', + + 'lib/test/angular/1.3.6/angular.js', + 'lib/test/angular/1.3.6/angular-mocks.js', + 'lib/test/angular/1.3.6/angular-animate.js', + + 'src/js/core/bootstrap.js', + 'src/js/**/*.js', + 'src/features/**/js/**/*.js', + 'test/unit/**/*.spec.js', + 'src/features/**/test/**/*.spec.js', + + 'dist/release/ui-grid.css', + + '.tmp/template.js' //templates + ], + + + // list of files to exclude + exclude: [ + ], + + + // test results reporter to use + // possible values: 'dots', 'progress', 'junit', 'growl', 'coverage' + reporters: ['dots', 'coverage'], + + preprocessors: { + // Cover source files but ignore any .spec.js files. Thanks goodness I found the pattern here: https://github.com/karma-runner/karma/pull/834#issuecomment-35626132 + 'src/**/!(*.spec)+(.js)': ['coverage'] + }, + + coverageReporter: { + type: 'lcov', + dir: 'coverage', + subdir: '.' + }, + + // web server port + port: 9876, + + + // enable / disable colors in the output (reporters and logs) + colors: true, + + + // level of logging + // possible values: config.LOG_DISABLE || config.LOG_ERROR || config.LOG_WARN || config.LOG_INFO || config.LOG_DEBUG + logLevel: config.LOG_INFO, + + + // enable / disable watching file and executing tests whenever any file changes + autoWatch: false, + + + // Start these browsers, currently available: + // - Chrome + // - ChromeCanary + // - Firefox + // - Opera (has to be installed with `npm install karma-opera-launcher`) + // - Safari (only Mac; has to be installed with `npm install karma-safari-launcher`) + // - PhantomJS + // - IE (only Windows; has to be installed with `npm install karma-ie-launcher`) + browsers: ['PhantomJS'], + + + // If browser does not capture in given timeout [ms], kill it + captureTimeout: 60000, + + + // Continuous Integration mode + // if true, it capture browsers, run tests and exit + singleRun: false, + + browserDisconnectTimeout: 10000, + browserDisconnectTolerance: 2, + browserNoActivityTimeout: 45000, // 20000, + + sauceLabs: { + username: 'nggrid', + startConnect: false, + testName: 'ui-grid unit tests' + }, + + // For more browsers on Sauce Labs see: + // https://saucelabs.com/docs/platforms/webdriver + customLaunchers: util.customLaunchers() + + }); + + // TODO(c0bra): remove once SauceLabs supports websockets. + // This speeds up the capturing a bit, as browsers don't even try to use websocket. -- (thanks vojta) + if (process.env.TRAVIS) { + config.logLevel = config.LOG_INFO; + config.browserNoActivityTimeout = 120000; // NOTE: from angular.js, for socket.io buffer + config.reporters = ['dots', 'coverage']; + + var buildLabel = 'TRAVIS #' + process.env.TRAVIS_BUILD_NUMBER + ' (' + process.env.TRAVIS_BUILD_ID + ')'; + + // config.transports = ['websocket', 'xhr-polling']; + + config.sauceLabs.build = buildLabel; + config.sauceLabs.startConnect = false; + config.sauceLabs.tunnelIdentifier = process.env.TRAVIS_JOB_NUMBER; + + config.transports = ['xhr-polling']; + + // Debug logging into a file, that we print out at the end of the build. + config.loggers.push({ + type: 'file', + filename: process.env.LOGS_DIR + '/' + ('karma.log') + }); + + + // NOTE: From angular.js project, only applies to SauceLabs -- (thanks Vojta again) + // Allocating a browser can take pretty long (eg. if we are out of capacity and need to wait + // for another build to finish) and so the `captureTimeout` typically kills + // an in-queue-pending request, which makes no sense. + config.captureTimeout = 0; + } + + if (grunt.option('browsers')) { + var bs = grunt.option('browsers').split(/,/).map(function(b) { return b.trim(); }); + config.browsers = bs; + } +}; \ No newline at end of file diff --git a/test/unit/core/factories/Grid.spec.js b/test/unit/core/factories/Grid.spec.js index b7a5030af0..4e1361b520 100644 --- a/test/unit/core/factories/Grid.spec.js +++ b/test/unit/core/factories/Grid.spec.js @@ -396,6 +396,38 @@ describe('Grid factory', function () { expect(grid1.columns[4].name).toEqual('5'); expect(grid1.columns[5].name).toEqual('5.5'); }); + + describe('when adding the same field multiple times', function () { + var grid; + + beforeEach(function () { + grid = new Grid({ id: 1 }); + grid.options.columnDefs = [{ field: 'a' }]; + grid.buildColumns(); + }); + + it('should not throw an exception', function () { + expect(function () { + for (var i = 1; i<=4; i++) { + grid.options.columnDefs.push({ field: 'a' }); + grid.buildColumns(); + } + }).not.toThrow(); + }); + + it('should create incremental displayNames', function () { + for (var i = 1; i<=4; i++) { + grid.options.columnDefs.push({ field: 'a' }); + } + grid.buildColumns(); + + expect(grid.columns[0].displayName).toEqual('A'); + expect(grid.columns[1].displayName).toEqual('A2'); + expect(grid.columns[2].displayName).toEqual('A3'); + expect(grid.columns[3].displayName).toEqual('A4'); + expect(grid.columns[4].displayName).toEqual('A5'); + }); + }); }); describe('follow source array', function() {