fix(config): increase test coverage

Also removes some dead code that isn't covered because it's dead.

lib/config/index.js

@@ -30,7 +30,6 @@ const configDefinition = definition()
  */
 const AZURE_APP_NAME = 'APP_POOL_ID'
 const DEFAULT_MAX_PAYLOAD_SIZE_IN_BYTES = 1000000
-const DEFAULT_CONFIG_PATH = require.resolve('./default')
 const BASE_CONFIG_PATH = require.resolve('../../newrelic')
 const HAS_ARBITRARY_KEYS = new Set(['ignore_messages', 'expected_messages', 'labels'])
 
@@ -75,11 +74,7 @@ function Config(config) {
   EventEmitter.call(this)
 
   // 1. start by cloning the defaults
-  try {
-    Object.assign(this, defaultConfig())
-  } catch (err) {
-    logger.warn('Unable to clone the default config, %s: %s', DEFAULT_CONFIG_PATH, err)
-  }
+  Object.assign(this, defaultConfig())
 
   // 2. initialize undocumented, internal-only default values
 
@@ -726,11 +721,6 @@ function _validateThenUpdateErrorMessages(remote, local, remoteKey, localKey) {
     return
   }
 
-  if (!valueToTest) {
-    logger.warn('SSC ignore|expect_message is null or undefined, will not merge')
-    return
-  }
-
   let valid = true
   Object.keys(valueToTest).forEach(function validateArray(key) {
     const arrayToTest = valueToTest[key]
@@ -958,10 +948,6 @@ Config.prototype.applications = function applications() {
     return apps
   }
 
-  if (apps && typeof apps === 'string') {
-    return [apps]
-  }
-
   return []
 }
 

test/unit/config/config-server-side.test.js

@@ -37,6 +37,13 @@ tap.test('when receiving server-side configuration', (t) => {
     t.end()
   })
 
+  t.test('should set the entity GUID', (t) => {
+    config.onConnect({ entity_guid: 1729 })
+    t.equal(config.entity_guid, 1729)
+
+    t.end()
+  })
+
   t.test('should set the application ID', (t) => {
     config.onConnect({ application_id: 76543 })
     t.equal(config.application_id, 76543)
@@ -241,10 +248,22 @@ tap.test('when receiving server-side configuration', (t) => {
   })
 
   t.test('should not blow up when trusted_account_ids is received', (t) => {
+    config.once('trusted_account_ids', (value) => {
+      t.same(value, [1, 2, 3], 'should get the initial keys')
+    })
+
     t.doesNotThrow(() => {
       config.onConnect({ trusted_account_ids: [1, 2, 3] })
+    }, 'should allow it once')
+
+    config.once('trusted_account_ids', (value) => {
+      t.same(value, [2, 3, 4], 'should get the modified keys')
     })
 
+    t.doesNotThrow(() => {
+      config.onConnect({ trusted_account_ids: [2, 3, 4] })
+    }, 'should allow modification')
+
     t.end()
   })
 
@@ -305,6 +324,14 @@ tap.test('when receiving server-side configuration', (t) => {
     t.end()
   })
 
+  t.test('should not blow up when browser_monitoring.loader is received', (t) => {
+    t.doesNotThrow(() => {
+      config.onConnect({ 'browser_monitoring.loader': 'none' })
+    })
+
+    t.end()
+  })
+
   t.test('should not blow up when beacon is received', (t) => {
     t.doesNotThrow(() => {
       config.onConnect({ beacon: 'beacon-0.newrelic.com' })
@@ -313,7 +340,7 @@ tap.test('when receiving server-side configuration', (t) => {
     t.end()
   })
 
-  t.test('should not blow up when beacon is received', (t) => {
+  t.test('should not blow up when error beacon is received', (t) => {
     t.doesNotThrow(() => {
       config.onConnect({ error_beacon: null })
     })
@@ -371,6 +398,16 @@ tap.test('when receiving server-side configuration', (t) => {
     t.end()
   })
 
+  t.test('should not blow up when collect_custom_events is received', (t) => {
+    config.custom_insights_events.enabled = true
+    t.doesNotThrow(() => {
+      config.onConnect({ collect_custom_events: false })
+    })
+    t.equal(config.custom_insights_events.enabled, false)
+
+    t.end()
+  })
+
   t.test('should not blow up when transaction_events.enabled is received', (t) => {
     t.doesNotThrow(() => {
       config.onConnect({ 'transaction_events.enabled': false })
@@ -442,6 +479,17 @@ tap.test('when receiving server-side configuration', (t) => {
       t.end()
     })
 
+    t.test('should not error out when ignore status codes are neither numbers nor strings', (t) => {
+      config.onConnect({
+        agent_config: {
+          'error_collector.ignore_status_codes': [{ non: 'sense' }]
+        }
+      })
+      t.same(config.error_collector.ignore_status_codes, [404])
+
+      t.end()
+    })
+
     t.test('should not add codes that parse to NaN', (t) => {
       config.onConnect({
         agent_config: {
@@ -558,6 +606,25 @@ tap.test('when receiving server-side configuration', (t) => {
       config.onConnect({ event_harvest_config: expectedHarvestConfig })
     })
 
+    t.test('should emit null when an invalid report period is provided', (t) => {
+      const invalidHarvestConfig = {
+        report_period_ms: -1,
+        harvest_limits: {
+          analytic_event_data: -1,
+          custom_event_data: -1,
+          error_event_data: -1
+        }
+      }
+
+      config.once('event_harvest_config', function (harvestconfig) {
+        t.same(harvestconfig, null, 'emitted value should be null')
+
+        t.end()
+      })
+
+      config.onConnect({ event_harvest_config: invalidHarvestConfig })
+    })
+
     t.test('should update event_harvest_config when a sub-value changed', (t) => {
       const originalHarvestConfig = {
         report_period_ms: 60000,
@@ -587,6 +654,43 @@ tap.test('when receiving server-side configuration', (t) => {
 
       config.onConnect({ event_harvest_config: expectedHarvestConfig })
     })
+
+    t.test('should ignore invalid limits on event_harvest_config', (t) => {
+      const originalHarvestConfig = {
+        report_period_ms: 60000,
+        harvest_limits: {
+          analytic_event_data: 10000,
+          custom_event_data: 10000,
+          error_event_data: 100
+        }
+      }
+
+      config.event_harvest_config = originalHarvestConfig
+
+      const invalidHarvestLimits = {
+        report_period_ms: 60000,
+        harvest_limits: {
+          analytic_event_data: -1,
+          custom_event_data: -1,
+          error_event_data: 200
+        }
+      }
+
+      const cleanedHarvestLimits = {
+        report_period_ms: 60000,
+        harvest_limits: {
+          error_event_data: 200
+        }
+      }
+
+      config.once('event_harvest_config', function (harvestconfig) {
+        t.same(harvestconfig, cleanedHarvestLimits, 'should not include invalid limits')
+
+        t.end()
+      })
+
+      config.onConnect({ event_harvest_config: invalidHarvestLimits })
+    })
   })
 
   t.test('when apdex_t is set', (t) => {

test/unit/error_events.test.js

@@ -27,8 +27,14 @@ test('Error events', (t) => {
 
     t.test('collector can override', (t) => {
       agent.config.error_collector.capture_events = false
-      t.doesNotThrow(() => agent.config.onConnect({ 'error_collector.capture_events': true }))
+      t.doesNotThrow(() =>
+        agent.config.onConnect({
+          'error_collector.capture_events': true,
+          'error_collector.max_event_samples_stored': 42
+        })
+      )
       t.equal(agent.config.error_collector.capture_events, true)
+      t.equal(agent.config.error_collector.max_event_samples_stored, 42)
 
       t.end()
     })

test/unit/errors/server-config.test.js

@@ -65,6 +65,18 @@ describe('Server Config', function () {
       })
     })
 
+    it('_fromServer should skip over malformed ignore_classes', function () {
+      helper.runInTransaction(agent, function () {
+        agent.config.error_collector.ignore_classes = ['Foo']
+        const params = { 'error_collector.ignore_classes': ['Bar'] }
+        agent.config._fromServer(params, 'error_collector.ignore_classes')
+        const nonsense = { 'error_collector.ignore_classes': [{ this: 'isNotAClass' }] }
+        agent.config._fromServer(nonsense, 'error_collector.ignore_classes')
+        const expected = ['Foo', 'Bar']
+        expect(agent.config.error_collector.ignore_classes).eql(expected)
+      })
+    })
+
     it('_fromServer should update expected_messages', function () {
       helper.runInTransaction(agent, function () {
         agent.config.error_collector.expected_messages = { Foo: ['bar'] }

test/unit/facts.test.js

@@ -651,6 +651,14 @@ tap.test('display_host', { timeout: 20000 }, (t) => {
     })
   })
 
+  t.test('should change large hostname of more than 255 bytes to safe value', (t) => {
+    agent.config.process_host.display_name = 'lo'.repeat(200)
+    facts(agent, function getFacts(factsed) {
+      t.equal(factsed.display_host, agent.config.getHostnameSafe())
+      t.end()
+    })
+  })
+
   t.test('should be process.env.DYNO when use_heroku_dyno_names is true', (t) => {
     process.env.DYNO = 'web.1'
     agent.config.heroku.use_dyno_names = true