Chore: Allow load metrics emission without file info (#828)

box · Aug 31, 2018 · 863be29 · 863be29
1 parent 55b4d8e
commit 863be29
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 31 deletions.
diff --git a/src/lib/Preview.js b/src/lib/Preview.js
@@ -1528,15 +1528,8 @@ class Preview extends EventEmitter {
         const downloadTag = Timer.createTag(this.file.id, LOAD_METRIC.downloadResponseTime);
         const fullLoadTag = Timer.createTag(this.file.id, LOAD_METRIC.fullDocumentLoadTime);
 
-        // Do nothing if there is nothing worth logging.
-        const infoTime = Timer.get(infoTag) || {};
-        if (!infoTime.elapsed) {
-            Timer.reset([infoTag, convertTag, downloadTag, fullLoadTag]);
-            return;
-        }
-
         const timerList = [
-            infoTime,
+            Timer.get(infoTag) || {},
             Timer.get(convertTag) || {},
             Timer.get(downloadTag) || {},
             Timer.get(fullLoadTag) || {}

diff --git a/src/lib/__tests__/Preview-test.js b/src/lib/__tests__/Preview-test.js
@@ -2305,7 +2305,7 @@ describe('lib/Preview', () => {
             Timer.reset();
         });
 
-        it('should do nothingescape early if no file or file id', () => {
+        it('should do nothing and escape early if no file or file id', () => {
             sandbox.stub(Timer, 'reset');
             sandbox.stub(preview, 'emit');
             preview.file = undefined;
@@ -2314,23 +2314,22 @@ describe('lib/Preview', () => {
             expect(preview.emit).to.not.be.called;
         });
 
-        it('should reset the timer and escape early if the first load milestone is not hit', () => {
-            Timer.reset(); // Clear out all entries in the Timer
-            sandbox.stub(Timer, 'reset');
-            sandbox.stub(preview, 'emit');
+        it('should emit a preview_metric event', (done) => {
+            preview.once(PREVIEW_METRIC, () => {
+                done();
+            });
             preview.emitLoadMetrics();
-            expect(Timer.reset).to.be.called;
-            expect(preview.emit).to.not.be.called;
         });
 
-        it('should emit a preview_metric event', (done) => {
-            preview.on(PREVIEW_METRIC, () => {
+        it('should emit a preview_metric event with event_name "load"', (done) => {
+            preview.once(PREVIEW_METRIC, (metric) => {
+                expect(metric.event_name).to.equal(LOAD_METRIC.previewLoadEvent);
                 done();
             });
             preview.emitLoadMetrics();
         });
 
-        it('should emit a preview_metric event with event_name "preview_load"', () => {
+        it('should emit a preview_metric event where the value property equals the sum of all load events', (done) => {
             const tag = Timer.createTag(preview.file.id, LOAD_METRIC.fullDocumentLoadTime);
             Timer.start(tag);
             Timer.stop(tag);
@@ -2340,25 +2339,20 @@ describe('lib/Preview', () => {
 
             const expectedTime = 30; // 10ms + 20ms
 
-            preview.on(PREVIEW_METRIC, (metric) => {
+            preview.once(PREVIEW_METRIC, (metric) => {
                 expect(metric.value).to.equal(expectedTime);
+                done();
             });
             preview.emitLoadMetrics();
         });
 
-        it('should emit a preview_metric event where the value property equals the sum of all load events', () => {
-            preview.on(PREVIEW_METRIC, (metric) => {
-                expect(metric.event_name).to.equal(LOAD_METRIC.previewLoadEvent);
-            });
-            preview.emitLoadMetrics();
-        });
-
-        it('should emit a preview_metric event with an object, with all of the proper load properties', () => {
-            preview.on(PREVIEW_METRIC, (metric) => {
+        it('should emit a preview_metric event with an object, with all of the proper load properties', (done) => {
+            preview.once(PREVIEW_METRIC, (metric) => {
                 expect(metric[LOAD_METRIC.fileInfoTime]).to.exist;
                 expect(metric[LOAD_METRIC.convertTime]).to.exist;
                 expect(metric[LOAD_METRIC.downloadResponseTime]).to.exist;
                 expect(metric[LOAD_METRIC.fullDocumentLoadTime]).to.exist;
+                done();
             });
             preview.emitLoadMetrics();
         });
@@ -2371,13 +2365,14 @@ describe('lib/Preview', () => {
             expect(preview.emit).to.be.called;
         });
 
-        it('should default all un-hit milestones, after the first, to 0, and cast float values to ints', () => {
+        it('should default all un-hit milestones, after the first, to 0, and cast float values to ints', (done) => {
             Timer.get(fileInfoTag).elapsed = 1.00001236712394687;
-            preview.on(PREVIEW_METRIC, (metric) => {
+            preview.once(PREVIEW_METRIC, (metric) => {
                 expect(metric[LOAD_METRIC.fileInfoTime]).to.equal(1); // Converted to int
                 expect(metric[LOAD_METRIC.convertTime]).to.equal(0);
                 expect(metric[LOAD_METRIC.downloadResponseTime]).to.equal(0);
                 expect(metric[LOAD_METRIC.fullDocumentLoadTime]).to.equal(0);
+                done();
             });
             preview.emitLoadMetrics();
         });

diff --git a/src/lib/events.js b/src/lib/events.js
@@ -44,7 +44,6 @@ export const ERROR_CODE = {
     FLASH_NOT_ENABLED: 'error_flash_not_enabled'
 };
 
-export const PREVIEW_LOAD_EVENT = '';
 // Event fired from Preview with error details
 export const PREVIEW_ERROR = 'preview_error';
 // Event fired from Preview with performance metrics