Wait for file stream to close before resolving promise (#133)

* Only resolve the promise once the writeStream has closed and finished

* Fix tests

* add create-packages

---------

Co-authored-by: Christian Holbrook <cholbrook@fubo.tv>

.eslintrc.js

@@ -130,6 +130,7 @@ module.exports = {
         'no-underscore-dangle': 'off',
         'no-unneeded-ternary': 'off',
         'no-useless-escape': 'off',
+        'no-void': 'off',
         'no-warning-comments': 'off',
         'object-curly-spacing': 'off',
         'object-property-newline': 'off',
@@ -178,4 +179,4 @@ module.exports = {
         }
     ],
     ignorePatterns: ['types']
-};
+};

.github/workflows/create-package.yml

@@ -0,0 +1,57 @@
+name: create-package
+on:
+  pull_request:
+    types: [labeled, unlabeled, synchronize]
+jobs:
+  create-package:
+    runs-on: ubuntu-latest
+    if: contains(github.event.pull_request.labels.*.name, 'create-package')
+    env:
+      GH_TOKEN: ${{ github.token }}
+    steps:
+      - uses: actions/checkout@master
+      - uses: actions/setup-node@master
+        with:
+          node-version: "14.19.0"
+      # Get a bot token so the bot's name shows up on all our actions
+      - name: Get Token From roku-ci-token Application
+        uses: tibdex/github-app-token@v1
+        id: generate-token
+        with:
+          app_id: ${{ secrets.BOT_APP_ID }}
+          private_key: ${{ secrets.BOT_PRIVATE_KEY }}
+      - run: echo "TOKEN=${{ steps.generate-token.outputs.token }}" >> $GITHUB_ENV
+      - name: Compute variables
+        run: |
+          CURRENT_VERSION=$(grep -o '\"version\": *\"[^\"]*\"' package.json | awk -F'\"' '{print $4}')
+          SANITIZED_BRANCH_NAME=$(echo "$GITHUB_HEAD_REF" | sed 's/[^0-9a-zA-Z-]/-/g')
+          BUILD_VERSION="$CURRENT_VERSION-$SANITIZED_BRANCH_NAME.$(date +%Y%m%d%H%M%S)"
+          NPM_PACKAGE_NAME=$(grep -o '\"name\": *\"[^\"]*\"' package.json | awk -F'\"' '{print $4}')
+          ARTIFACT_NAME=$(echo "$NPM_PACKAGE_NAME-$BUILD_VERSION.tgz" | tr '/' '-')
+          ARTIFACT_URL="${{ github.server_url }}/${{ github.repository }}/releases/download/v0.0.0-packages/${ARTIFACT_NAME}"
+
+          echo "BUILD_VERSION=$BUILD_VERSION" >> $GITHUB_ENV
+          echo "ARTIFACT_URL=$ARTIFACT_URL" >> $GITHUB_ENV
+
+      - run: npm ci
+      - run: npm version "$BUILD_VERSION" --no-git-tag-version
+      - run: npm pack
+
+      # create the release if not exist
+      - run: gh release create v0.0.0-packages --latest=false --prerelease --notes "catchall release for temp packages" -R ${{ github.repository }}
+        continue-on-error: true
+
+      # upload this artifact to the "packages" github release
+      - run: gh release upload v0.0.0-packages *.tgz -R ${{ github.repository }}
+
+      - name: Fetch build artifact
+        uses: actions/github-script@v7
+        with:
+          github-token: ${{ env.TOKEN }}
+          script: |
+              return github.rest.issues.createComment({
+                issue_number: context.issue.number,
+                owner: context.repo.owner,
+                repo: context.repo.repo,
+                body: "Hey there! I just built a new temporary npm package based on ${{ github.event.pull_request.head.sha }}. You can download it [here](${{ env.ARTIFACT_URL }}) or install it by running the following command: \n```bash\nnpm install ${{ env.ARTIFACT_URL }}\n```"
+              });

package.json

@@ -41,6 +41,7 @@
         "@types/micromatch": "^4.0.2",
         "@types/mocha": "^9.0.0",
         "@types/node": "^16.11.3",
+        "@types/q": "^1.5.8",
         "@types/request": "^2.47.0",
         "@types/sinon": "^10.0.4",
         "@types/xml2js": "^0.4.5",
@@ -52,6 +53,7 @@
         "eslint": "8.0.1",
         "mocha": "^9.1.3",
         "nyc": "^15.1.0",
+        "q": "^1.5.1",
         "rimraf": "^2.6.2",
         "sinon": "^11.1.2",
         "source-map-support": "^0.5.13",

src/RokuDeploy.spec.ts

@@ -1,6 +1,9 @@
 import * as assert from 'assert';
 import { expect } from 'chai';
 import * as fsExtra from 'fs-extra';
+import type { WriteStream, PathLike } from 'fs-extra';
+import * as fs from 'fs';
+import * as q from 'q';
 import * as path from 'path';
 import * as JSZip from 'jszip';
 import * as child_process from 'child_process';
@@ -12,14 +15,20 @@ import { util, standardizePath as s } from './util';
 import type { FileEntry, RokuDeployOptions } from './RokuDeployOptions';
 import { cwd, expectPathExists, expectPathNotExists, expectThrowsAsync, outDir, rootDir, stagingDir, tempDir, writeFiles } from './testUtils.spec';
 import { createSandbox } from 'sinon';
-import * as request from 'postman-request';
+import * as r from 'postman-request';
+import type * as requestType from 'request';
+const request = r as typeof requestType;
 
 const sinon = createSandbox();
 
 describe('index', () => {
     let rokuDeploy: RokuDeploy;
     let options: RokuDeployOptions;
 
+    let writeStreamPromise: Promise<WriteStream>;
+    let writeStreamDeferred: q.Deferred<WriteStream> & { isComplete: undefined | true };
+    let createWriteStreamStub: sinon.SinonStub;
+
     beforeEach(() => {
         rokuDeploy = new RokuDeploy();
         options = rokuDeploy.getOptions({
@@ -38,9 +47,24 @@ describe('index', () => {
         fsExtra.ensureDirSync(stagingDir);
         //most tests depend on a manifest file existing, so write an empty one
         fsExtra.outputFileSync(`${rootDir}/manifest`, '');
+
+        writeStreamDeferred = q.defer<WriteStream>() as any;
+        writeStreamPromise = writeStreamDeferred.promise as any;
+
+        //fake out the write stream function
+        createWriteStreamStub = sinon.stub(rokuDeploy.fsExtra, 'createWriteStream').callsFake((filePath: PathLike) => {
+            const writeStream = fs.createWriteStream(filePath);
+            writeStreamDeferred.resolve(writeStream);
+            writeStreamDeferred.isComplete = true;
+            return writeStream;
+        });
     });
 
     afterEach(() => {
+        if (createWriteStreamStub.called && !writeStreamDeferred.isComplete) {
+            writeStreamDeferred.reject('Deferred was never resolved...so rejecting in the afterEach');
+        }
+
         sinon.restore();
         //restore the original working directory
         process.chdir(cwd);
@@ -1873,13 +1897,8 @@ describe('index', () => {
     describe('takeScreenshot', () => {
         let onHandler: any;
         let screenshotAddress: any;
+
         beforeEach(() => {
-            //fake out the write stream function
-            sinon.stub(rokuDeploy.fsExtra, 'createWriteStream').callsFake((filePath: any) => {
-                fsExtra.outputFileSync(filePath, 'test');
-                screenshotAddress = filePath;
-                return null;
-            });
 
             //intercept the http request
             sinon.stub(request, 'get').callsFake(() => {
@@ -1890,7 +1909,11 @@ describe('index', () => {
                         });
                         return req;
                     },
-                    pipe: () => { }
+                    pipe: async () => {
+                        const writeStream = await writeStreamPromise;
+                        writeStream.write(Buffer.from('test-content'));
+                        writeStream.close();
+                    }
                 };
                 return req;
             });
@@ -3001,9 +3024,6 @@ describe('index', () => {
                 //do nothing, assume the dir gets created
             }) as any);
 
-            //fake out the write stream function
-            sinon.stub(rokuDeploy.fsExtra, 'createWriteStream').returns(null);
-
             //intercept the http request
             sinon.stub(request, 'get').callsFake(() => {
                 let req: any = {
@@ -3013,12 +3033,17 @@ describe('index', () => {
                         });
                         return req;
                     },
-                    pipe: () => { }
+                    pipe: async () => {
+                        //if a write stream gets created, write some stuff and close it
+                        const writeStream = await writeStreamPromise;
+                        writeStream.write('test');
+                        writeStream.close();
+                    }
                 };
                 return req;
             });
-
         });
+
         it('returns a pkg file path on success', async () => {
             onHandler = (event, callback) => {
                 if (event === 'response') {
@@ -3033,6 +3058,31 @@ describe('index', () => {
             expect(pkgFilePath).to.equal(path.join(process.cwd(), 'out', 'roku-deploy-test.pkg'));
         });
 
+        it('returns a pkg file path on success', async () => {
+            //the write stream should return null, which causes a specific branch to be executed
+            createWriteStreamStub.callsFake(() => {
+                return null;
+            });
+
+            onHandler = (event, callback) => {
+                if (event === 'response') {
+                    callback({
+                        statusCode: 200
+                    });
+                }
+            };
+
+            let error: Error;
+            try {
+                await rokuDeploy.retrieveSignedPackage('path_to_pkg', {
+                    outFile: 'roku-deploy-test'
+                });
+            } catch (e) {
+                error = e as any;
+            }
+            expect(error.message.startsWith('Unable to create write stream for')).to.be.true;
+        });
+
         it('throws when error in request is encountered', async () => {
             onHandler = (event, callback) => {
                 if (event === 'error') {
@@ -3353,6 +3403,52 @@ describe('index', () => {
         });
     });
 
+    describe('getToFile', () => {
+        it('waits for the write stream to finish writing before resolving', async () => {
+            let getToFileIsResolved = false;
+
+            let requestCalled = q.defer();
+            let onResponse = q.defer<(res) => any>();
+
+            //intercept the http request
+            sinon.stub(request, 'get').callsFake(() => {
+                requestCalled.resolve();
+                let req: any = {
+                    on: (event, callback) => {
+                        if (event === 'response') {
+                            onResponse.resolve(callback);
+                        }
+                        return req;
+                    },
+                    pipe: () => {
+                        return req;
+                    }
+                };
+                return req;
+            });
+
+            const finalPromise = rokuDeploy['getToFile']({}, s`${tempDir}/out/something.txt`).then(() => {
+                getToFileIsResolved = true;
+            });
+
+            await requestCalled.promise;
+            expect(getToFileIsResolved).to.be.false;
+
+            const callback = await onResponse.promise;
+            callback({ statusCode: 200 });
+            await util.sleep(10);
+
+            expect(getToFileIsResolved).to.be.false;
+
+            const writeStream = await writeStreamPromise;
+            writeStream.write('test');
+            writeStream.close();
+
+            await finalPromise;
+            expect(getToFileIsResolved).to.be.true;
+        });
+    });
+
     describe('deployAndSignPackage', () => {
         beforeEach(() => {
             //pretend the deploy worked