From bc7695ee2a27e9dc859b8365a60348ef4dd7d862 Mon Sep 17 00:00:00 2001
From: Stephen Palmer <stephenp@unity3d.com>
Date: Fri, 14 Jun 2019 10:13:54 -0500
Subject: [PATCH] - Fix client recorder and tests to not pollute the working
 directory with diagnostic files created for test runs - Don't write zero-byte
 diagnostic files

---
 lib/server/client_stream_recorder.js |  2 ++
 test/client_stream_recorder.js       | 21 ++++++++++++++++++++-
 test/server.js                       | 22 +++++++++++-----------
 test/unity_cache_server.js           |  3 ++-
 4 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/lib/server/client_stream_recorder.js b/lib/server/client_stream_recorder.js
index d7cf3b1..ff6eeaa 100644
--- a/lib/server/client_stream_recorder.js
+++ b/lib/server/client_stream_recorder.js
@@ -79,6 +79,8 @@ class ClientStreamRecorder extends Writable {
     }
 
     async _flush_buffer() {
+        if(this._bufferPos === 0) return;
+
         await fs.ensureDir(this._saveDir);
 
         // Normalize the version size so it will be correctly parsed when streamed to a server.
diff --git a/test/client_stream_recorder.js b/test/client_stream_recorder.js
index bf3c252..cc576ad 100644
--- a/test/client_stream_recorder.js
+++ b/test/client_stream_recorder.js
@@ -31,7 +31,7 @@ describe('ClientStreamRecorder', () => {
     it('should use options passed into constructor', () => {
         const opts = {
             sessionId: uuid.v4(),
-            saveDir: 'temp',
+            saveDir: this.tmpDir.name,
             bufferSize: Math.floor(Math.random() * 100000)
         };
 
@@ -41,6 +41,25 @@ describe('ClientStreamRecorder', () => {
         assert.strictEqual(csr._bufferSize, opts.bufferSize);
     });
 
+    it('should not write diagnostic file if client did not send any data', async () => {
+        const opts = {
+            saveDir: this.tmpDir.name,
+            bufferSize: 1024
+        };
+
+        const csr = new ClientStreamRecorder(opts);
+        const csrWrite = promisify(csr.write).bind(csr);
+        const dataPath = csr.dataPath;
+
+        await csrWrite(Buffer.alloc(0));
+        assert(!await fs.pathExists(dataPath));
+        csr.emit('unpipe'); // triggers a buffer flush
+        await csrWrite(Buffer.alloc(1025));
+        csr.emit('unpipe');
+        assert(await fs.pathExists(dataPath));
+    });
+
+
     it('should not write to disk until the internal buffer is full', async () => {
         const opts = {
             saveDir: this.tmpDir.name,
diff --git a/test/server.js b/test/server.js
index a4dfa60..75d0cd5 100644
--- a/test/server.js
+++ b/test/server.js
@@ -9,7 +9,7 @@ const CacheServer = require('../lib/server/server');
 const tmp = require('tmp');
 const ClientStreamRecorder = require('../lib/server/client_stream_recorder');
 const CacheBase = require('../lib/cache/cache_base').CacheBase;
-const { generateCommandData, encodeCommand, clientWrite, sleep, cmd } = require('./test_utils');
+const { generateCommandData, encodeCommand, clientWrite, sleep, cmd, purgeConfig } = require('./test_utils');
 const sinon = require('sinon');
 
 const cache = new CacheBase();
@@ -223,16 +223,17 @@ describe("Server common", function() {
         before(async () => {
             this.tmpDir = tmp.dirSync({unsafeCleanup: true});
 
-            const serverOpts = {
-                port: 0,
-                clientRecorder: {
-                    enabled: true,
-                    saveDir: this.tmpDir.name,
-                    bufferSize: 1024
+            purgeConfig();
+            process.env.NODE_CONFIG = JSON.stringify({
+                Diagnostics: {
+                    clientRecorderOptions: {
+                        saveDir: this.tmpDir.name,
+                        bufferSize: 1024
+                    }
                 }
-            };
+            });
 
-            this.csrServer = new CacheServer(cache, serverOpts);
+            this.csrServer = new CacheServer(cache, {clientRecorder: true});
             return this.csrServer.start(err => assert(!err, `Cache Server reported error! ${err}`));
         });
 
@@ -247,13 +248,12 @@ describe("Server common", function() {
             this.csrServer.server.on('connection', socket => {
                 assert.ok(Array.isArray(socket._readableState.pipes));
                 assert.ok(socket._readableState.pipes.find(x => x instanceof ClientStreamRecorder));
-                done();
             });
 
             client = net.connect({port: this.csrServer.port}, () => {
                 client.write(helpers.encodeInt32(consts.PROTOCOL_VERSION));
                 client.end(cmd.quit);
-            });
+            }).on('data', () => {}).on('close', () => done());
         });
     });
 
diff --git a/test/unity_cache_server.js b/test/unity_cache_server.js
index b0001ac..a1e1289 100644
--- a/test/unity_cache_server.js
+++ b/test/unity_cache_server.js
@@ -213,7 +213,8 @@ describe("Unity Cache Server bootstrap", () => {
                     }
                 },
                 Diagnostics: {
-                    clientRecorder: true
+                    clientRecorder: true,
+                    saveDir: tmpPath
 				}
             });