Respond to comments: include check that values are not blank, and adjust test format.

Author: kim-f

packages/performance/src/resources/trace.test.ts

@@ -117,13 +117,13 @@ describe('Firebase Performance > trace', () => {
       expect(trace.getMetric('cacheHits')).to.eql(600);
     });
 
-    it("throws error if metric doesn't exist and has invalid name", () => {
+    it('throws error if metric doesn\'t exist and has invalid name', () => {
       expect(() => trace.incrementMetric('_invalidMetric', 1)).to.throw();
     });
   });
 
   describe('#putMetric', () => {
-    it('creates new metric if one doesnt exist and has valid name.', () => {
+    it('creates new metric if one doesn\'t exist and has valid name.', () => {
       trace.putMetric('cacheHits', 200);
 
       expect(trace.getMetric('cacheHits')).to.eql(200);
@@ -136,7 +136,7 @@ describe('Firebase Performance > trace', () => {
       expect(trace.getMetric('cacheHits')).to.eql(400);
     });
 
-    it("throws error if metric doesn't exist and has invalid name", () => {
+    it('throws error if metric doesn\'t exist and has invalid name', () => {
       expect(() => trace.putMetric('_invalidMetric', 1)).to.throw();
     });
   });

packages/performance/src/utils/attribute_utils.test.ts

@@ -176,15 +176,15 @@ describe('Firebase Performance > attribute_utils', () => {
   });
 
   describe('#isValidCustomAttributeName', () => {
-    afterEach(() => {
-      restore();
-    });
-
     it('returns true when name is valid', () => {
       expect(isValidCustomAttributeName('validCustom_Attribute_Name')).to.be
         .true;
     });
 
+    it('returns false when name is blank', () => {
+      expect(isValidCustomAttributeName('')).to.be.false;
+    });
+
     it('returns false when name is too long', () => {
       expect(
         isValidCustomAttributeName('invalid_custom_name_over_forty_characters')
@@ -206,14 +206,14 @@ describe('Firebase Performance > attribute_utils', () => {
   });
 
   describe('#isValidCustomAttributeValue', () => {
-    afterEach(() => {
-      restore();
-    });
-
     it('returns true when value is valid', () => {
       expect(isValidCustomAttributeValue('valid_attribute_value')).to.be.true;
     });
 
+    it('returns false when value is blank', () => {
+      expect(isValidCustomAttributeValue('')).to.be.false;
+    });
+
     it('returns false when value is too long', () => {
       const longAttributeValue =
         'too_long_attribute_value_over_one_hundred_characters_too_long_attribute_value_over_one_' +

packages/performance/src/utils/attributes_utils.ts

@@ -95,7 +95,7 @@ export function getEffectiveConnectionType(): EffectiveConnectionType {
 }
 
 export function isValidCustomAttributeName(name: string): boolean {
-  if (name.length > MAX_ATTRIBUTE_NAME_LENGTH) {
+  if (name.length == 0 || name.length > MAX_ATTRIBUTE_NAME_LENGTH) {
     return false;
   }
   const matchesReservedPrefix = RESERVED_ATTRIBUTE_PREFIXES.some(prefix =>
@@ -105,5 +105,5 @@ export function isValidCustomAttributeName(name: string): boolean {
 }
 
 export function isValidCustomAttributeValue(value: string): boolean {
-  return value.length <= MAX_ATTRIBUTE_VALUE_LENGTH;
+  return value.length != 0 && value.length <= MAX_ATTRIBUTE_VALUE_LENGTH;
 }

packages/performance/src/utils/metric_utils.test.ts

@@ -15,7 +15,6 @@
  * limitations under the License.
  */
 
-import { restore } from 'sinon';
 import { expect } from 'chai';
 
 import { isValidCustomMetricName } from './metric_utils';
@@ -24,14 +23,14 @@ import '../../test/setup';
 
 describe('Firebase Performance > metric_utils', () => {
   describe('#isValidCustomMetricName', () => {
-    afterEach(() => {
-      restore();
-    });
-
     it('returns true when name is valid', () => {
       expect(isValidCustomMetricName('validCustom_Metric_Name')).to.be.true;
     });
 
+    it('returns false when name is blank', () => {
+      expect(isValidCustomMetricName('')).to.be.false;
+    });
+
     it('returns false when name is too long', () => {
       const longMetricName =
         'too_long_metric_name_over_one_hundred_characters_too_long_metric_name_over_one_' +

packages/performance/src/utils/metric_utils.ts

@@ -19,7 +19,7 @@ const MAX_METRIC_NAME_LENGTH = 100;
 const RESERVED_AUTO_PREFIX = '_';
 
 export function isValidCustomMetricName(name: string): boolean {
-  if (name.length > MAX_METRIC_NAME_LENGTH) {
+  if (name.length == 0 || name.length > MAX_METRIC_NAME_LENGTH) {
     return false;
   }
   return !name.startsWith(RESERVED_AUTO_PREFIX);