test(aio): add e2e tests for non-public previews

aio/aio-builds-setup/dockerbuild/scripts-js/lib/upload-server/index-test.ts

@@ -1,10 +1,24 @@
 // Imports
 import {GithubPullRequests} from '../common/github-pull-requests';
 import {BUILD_VERIFICATION_STATUS, BuildVerifier} from './build-verifier';
+import {UploadError} from './upload-error';
 
 // Run
 // TODO(gkalpak): Add e2e tests to cover these interactions as well.
 GithubPullRequests.prototype.addComment = () => Promise.resolve();
-BuildVerifier.prototype.verify = () => Promise.resolve(BUILD_VERIFICATION_STATUS.verifiedAndTrusted);
+BuildVerifier.prototype.verify = (expectedPr: number, authHeader: string) => {
+  switch (authHeader) {
+    case 'FAKE_VERIFICATION_ERROR':
+      // For e2e tests, fake a verification error.
+      return Promise.reject(new UploadError(403, `Error while verifying upload for PR ${expectedPr}: Test`));
+    case 'FAKE_VERIFIED_NOT_TRUSTED':
+      // For e2e tests, fake a `verifiedNotTrusted` verification status.
+      return Promise.resolve(BUILD_VERIFICATION_STATUS.verifiedNotTrusted);
+    default:
+      // For e2e tests, default to `verifiedAndTrusted` verification status.
+      return Promise.resolve(BUILD_VERIFICATION_STATUS.verifiedAndTrusted);
+  }
+};
+
 // tslint:disable-next-line: no-var-requires
 require('./index');

aio/aio-builds-setup/dockerbuild/scripts-js/lib/verify-setup/helper.ts

@@ -5,6 +5,7 @@ import * as http from 'http';
 import * as path from 'path';
 import * as shell from 'shelljs';
 import {getEnvVar} from '../common/utils';
+import {BuildCreator} from '../upload-server/build-creator';
 
 // Constans
 const TEST_AIO_BUILDS_DIR = getEnvVar('TEST_AIO_BUILDS_DIR');
@@ -31,10 +32,10 @@ class Helper {
   public get nginxHostname() { return TEST_AIO_NGINX_HOSTNAME; }
   public get nginxPortHttp() { return TEST_AIO_NGINX_PORT_HTTP; }
   public get nginxPortHttps() { return TEST_AIO_NGINX_PORT_HTTPS; }
-  public get wwwUser() { return WWW_USER; }
   public get uploadHostname() { return TEST_AIO_UPLOAD_HOSTNAME; }
   public get uploadPort() { return TEST_AIO_UPLOAD_PORT; }
   public get uploadMaxSize() { return TEST_AIO_UPLOAD_MAX_SIZE; }
+  public get wwwUser() { return WWW_USER; }
 
   // Properties - Protected
   protected cleanUpFns: CleanUpFn[] = [];
@@ -50,6 +51,11 @@ class Helper {
   }
 
   // Methods - Public
+  public buildExists(pr: string, sha = '', isPublic = true): boolean {
+    const dir = path.join(this.getPrDir(pr, isPublic), sha);
+    return fs.existsSync(dir);
+  }
+
   public cleanUp() {
     while (this.cleanUpFns.length) {
       // Clean-up fns remove themselves from the list.
@@ -66,7 +72,7 @@ class Helper {
     const cmd1 = `tar --create --gzip --directory "${inputDir}" --file "${archivePath}" .`;
     const cmd2 = `chown ${this.wwwUser} ${archivePath}`;
 
-    const cleanUpTemp = this.createDummyBuild(`uploaded/${pr}`, sha, true);
+    const cleanUpTemp = this.createDummyBuild(`uploaded/${pr}`, sha, true, true);
     shell.exec(cmd1);
     shell.exec(cmd2);
     cleanUpTemp();
@@ -74,8 +80,8 @@ class Helper {
     return this.createCleanUpFn(() => shell.rm('-rf', archivePath));
   }
 
-  public createDummyBuild(pr: string, sha: string, force = false): CleanUpFn {
+  public createDummyBuild(pr: string, sha: string, isPublic = true, force = false): CleanUpFn {
-    const prDir = path.join(this.buildsDir, pr);
+    const prDir = this.getPrDir(pr, isPublic);
     const shaDir = path.join(prDir, sha);
     const idxPath = path.join(shaDir, 'index.html');
     const barPath = path.join(shaDir, 'foo', 'bar.js');
@@ -87,8 +93,8 @@ class Helper {
     return this.createCleanUpFn(() => shell.rm('-rf', prDir));
   }
 
-  public deletePrDir(pr: string) {
+  public deletePrDir(pr: string, isPublic = true) {
-    const prDir = path.join(this.buildsDir, pr);
+    const prDir = this.getPrDir(pr, isPublic);
 
     if (fs.existsSync(prDir)) {
       // Undocumented signature (see https://github.com/shelljs/shelljs/pull/663).
@@ -97,8 +103,14 @@ class Helper {
     }
   }
 
-  public readBuildFile(pr: string, sha: string, relFilePath: string): string {
+  public getPrDir(pr: string, isPublic: boolean): string {
-    const absFilePath = path.join(this.buildsDir, pr, sha, relFilePath);
+    const prDirName = isPublic ? pr : BuildCreator.HIDDEN_DIR_PREFIX + pr;
+    return path.join(this.buildsDir, prDirName);
+  }
+
+  public readBuildFile(pr: string, sha: string, relFilePath: string, isPublic = true): string {
+    const prDir = this.getPrDir(pr, isPublic);
+    const absFilePath = path.join(prDir, sha, relFilePath);
     return fs.readFileSync(absFilePath, 'utf8');
   }
 
@@ -129,7 +141,8 @@ class Helper {
       const [headers, body] = result.stdout.
         split(/(?:\r?\n){2,}/).
         map(s => s.trim()).
-        slice(-2);
+        slice(-2);   // In case of redirect, discard the previous headers.
+                     // Only keep the last to sections (final headers and body).
 
       if (!result.success) {
         console.log('Stdout:', result.stdout);
@@ -143,8 +156,8 @@ class Helper {
     };
   }
 
-  public writeBuildFile(pr: string, sha: string, relFilePath: string, content: string): CleanUpFn {
+  public writeBuildFile(pr: string, sha: string, relFilePath: string, content: string, isPublic = true): CleanUpFn {
-    const absFilePath = path.join(this.buildsDir, pr, sha, relFilePath);
+    const absFilePath = path.join(this.getPrDir(pr, isPublic), sha, relFilePath);
     return this.writeFile(absFilePath, {content}, true);
   }
 

aio/aio-builds-setup/dockerbuild/scripts-js/lib/verify-setup/nginx.e2e.ts

@@ -31,7 +31,7 @@ describe(`nginx`, () => {
   });
 
 
-  h.runForAllSupportedSchemes((scheme, port) => describe(`nginx (on ${scheme.toUpperCase()})`, () => {
+  h.runForAllSupportedSchemes((scheme, port) => describe(`(on ${scheme.toUpperCase()})`, () => {
     const hostname = h.nginxHostname;
     const host = `${hostname}:${port}`;
     const pr = '9';
@@ -41,6 +41,8 @@ describe(`nginx`, () => {
 
     describe(`pr<pr>-<sha>.${host}/*`, () => {
 
+      describe('(for public builds)', () => {
+
         beforeEach(() => {
           h.createDummyBuild(pr, sha9);
           h.createDummyBuild(pr, sha0);
@@ -141,6 +143,30 @@ describe(`nginx`, () => {
       });
 
 
+      describe('(for hidden builds)', () => {
+
+        beforeEach(() => h.createDummyBuild(pr, sha9, false));
+
+
+        it('should respond with 404 for any file or directory', done => {
+          const origin = `${scheme}://pr${pr}-${sha9}.${host}`;
+          const assert404 = h.verifyResponse(404);
+
+          Promise.all([
+            h.runCmd(`curl -iL ${origin}/index.html`).then(assert404),
+            h.runCmd(`curl -iL ${origin}/`).then(assert404),
+            h.runCmd(`curl -iL ${origin}`).then(assert404),
+            h.runCmd(`curl -iL ${origin}/foo/bar.js`).then(assert404),
+            h.runCmd(`curl -iL ${origin}/foo/`).then(assert404),
+            h.runCmd(`curl -iL ${origin}/foo`).then(assert404),
+          ]).then(done);
+        });
+
+      });
+
+    });
+
+
     describe(`${host}/health-check`, () => {
 
       it('should respond with 200', done => {

aio/aio-builds-setup/dockerbuild/scripts-js/lib/verify-setup/server-integration.e2e.ts

@@ -13,19 +13,25 @@ h.runForAllSupportedSchemes((scheme, port) => describe(`integration (on ${scheme
 
   const getFile = (pr: string, sha: string, file: string) =>
     h.runCmd(`curl -iL ${scheme}://pr${pr}-${sha}.${host}/${file}`);
-  const uploadBuild = (pr: string, sha: string, archive: string) => {
+  const uploadBuild = (pr: string, sha: string, archive: string, authHeader = 'Token FOO') => {
-    const curlPost = 'curl -iLX POST --header "Authorization: Token FOO"';
+    // Using `FAKE_VERIFICATION_ERROR` or `FAKE_VERIFIED_NOT_TRUSTED` as `authHeader`,
+    // we can fake the response of the overwritten `BuildVerifier.verify()` method.
+    // (See 'lib/upload-server/index-test.ts'.)
+    const curlPost = `curl -iLX POST --header "Authorization: ${authHeader}"`;
     return h.runCmd(`${curlPost} --data-binary "@${archive}" ${scheme}://${host}/create-build/${pr}/${sha}`);
   };
 
   beforeEach(() => jasmine.DEFAULT_TIMEOUT_INTERVAL = 10000);
   afterEach(() => {
     h.deletePrDir(pr9);
+    h.deletePrDir(pr9, false);
     h.cleanUp();
   });
 
 
-  it('should be able to upload and serve a build for a new PR', done => {
+  describe('for a new PR', () => {
+
+    it('should be able to upload and serve a public build', done => {
       const regexPrefix9 = `^PR: uploaded\\/${pr9} \\| SHA: ${sha9} \\| File:`;
       const idxContentRegex9 = new RegExp(`${regexPrefix9} \\/index\\.html$`);
       const barContentRegex9 = new RegExp(`${regexPrefix9} \\/foo\\/bar\\.js$`);
@@ -41,7 +47,48 @@ h.runForAllSupportedSchemes((scheme, port) => describe(`integration (on ${scheme
     });
 
 
-  it('should be able to upload and serve a build for an existing PR', done => {
+    it('should be able to upload but not serve a hidden build', done => {
+      const regexPrefix9 = `^PR: uploaded\\/${pr9} \\| SHA: ${sha9} \\| File:`;
+      const idxContentRegex9 = new RegExp(`${regexPrefix9} \\/index\\.html$`);
+      const barContentRegex9 = new RegExp(`${regexPrefix9} \\/foo\\/bar\\.js$`);
+
+      h.createDummyArchive(pr9, sha9, archivePath);
+
+      uploadBuild(pr9, sha9, archivePath, 'FAKE_VERIFIED_NOT_TRUSTED').
+        then(() => Promise.all([
+          getFile(pr9, sha9, 'index.html').then(h.verifyResponse(404)),
+          getFile(pr9, sha9, 'foo/bar.js').then(h.verifyResponse(404)),
+        ])).
+        then(() => {
+          expect(h.buildExists(pr9, sha9)).toBe(false);
+          expect(h.buildExists(pr9, sha9, false)).toBe(true);
+          expect(h.readBuildFile(pr9, sha9, 'index.html', false)).toMatch(idxContentRegex9);
+          expect(h.readBuildFile(pr9, sha9, 'foo/bar.js', false)).toMatch(barContentRegex9);
+        }).
+        then(done);
+    });
+
+
+    it('should reject an upload if verification fails', done => {
+      const errorRegex9 = new RegExp(`Error while verifying upload for PR ${pr9}: Test`);
+
+      h.createDummyArchive(pr9, sha9, archivePath);
+
+      uploadBuild(pr9, sha9, archivePath, 'FAKE_VERIFICATION_ERROR').
+        then(h.verifyResponse(403, errorRegex9)).
+        then(() => {
+          expect(h.buildExists(pr9)).toBe(false);
+          expect(h.buildExists(pr9, '', false)).toBe(false);
+        }).
+        then(done);
+    });
+
+  });
+
+
+  describe('for an existing PR', () => {
+
+    it('should be able to upload and serve a public build', done => {
       const regexPrefix0 = `^PR: ${pr9} \\| SHA: ${sha0} \\| File:`;
       const idxContentRegex0 = new RegExp(`${regexPrefix0} \\/index\\.html$`);
       const barContentRegex0 = new RegExp(`${regexPrefix0} \\/foo\\/bar\\.js$`);
@@ -64,7 +111,56 @@ h.runForAllSupportedSchemes((scheme, port) => describe(`integration (on ${scheme
     });
 
 
-  it('should not be able to overwrite a build', done => {
+    it('should be able to upload but not serve a hidden build', done => {
+      const regexPrefix0 = `^PR: ${pr9} \\| SHA: ${sha0} \\| File:`;
+      const idxContentRegex0 = new RegExp(`${regexPrefix0} \\/index\\.html$`);
+      const barContentRegex0 = new RegExp(`${regexPrefix0} \\/foo\\/bar\\.js$`);
+
+      const regexPrefix9 = `^PR: uploaded\\/${pr9} \\| SHA: ${sha9} \\| File:`;
+      const idxContentRegex9 = new RegExp(`${regexPrefix9} \\/index\\.html$`);
+      const barContentRegex9 = new RegExp(`${regexPrefix9} \\/foo\\/bar\\.js$`);
+
+      h.createDummyBuild(pr9, sha0, false);
+      h.createDummyArchive(pr9, sha9, archivePath);
+
+      uploadBuild(pr9, sha9, archivePath, 'FAKE_VERIFIED_NOT_TRUSTED').
+        then(() => Promise.all([
+          getFile(pr9, sha0, 'index.html').then(h.verifyResponse(404)),
+          getFile(pr9, sha0, 'foo/bar.js').then(h.verifyResponse(404)),
+          getFile(pr9, sha9, 'index.html').then(h.verifyResponse(404)),
+          getFile(pr9, sha9, 'foo/bar.js').then(h.verifyResponse(404)),
+        ])).
+        then(() => {
+          expect(h.buildExists(pr9, sha9)).toBe(false);
+          expect(h.buildExists(pr9, sha9, false)).toBe(true);
+          expect(h.readBuildFile(pr9, sha0, 'index.html', false)).toMatch(idxContentRegex0);
+          expect(h.readBuildFile(pr9, sha0, 'foo/bar.js', false)).toMatch(barContentRegex0);
+          expect(h.readBuildFile(pr9, sha9, 'index.html', false)).toMatch(idxContentRegex9);
+          expect(h.readBuildFile(pr9, sha9, 'foo/bar.js', false)).toMatch(barContentRegex9);
+        }).
+        then(done);
+    });
+
+
+    it('should reject an upload if verification fails', done => {
+      const errorRegex9 = new RegExp(`Error while verifying upload for PR ${pr9}: Test`);
+
+      h.createDummyBuild(pr9, sha0);
+      h.createDummyArchive(pr9, sha9, archivePath);
+
+      uploadBuild(pr9, sha9, archivePath, 'FAKE_VERIFICATION_ERROR').
+        then(h.verifyResponse(403, errorRegex9)).
+        then(() => {
+          expect(h.buildExists(pr9)).toBe(true);
+          expect(h.buildExists(pr9, sha0)).toBe(true);
+          expect(h.buildExists(pr9, sha9)).toBe(false);
+        }).
+        then(done);
+
+    });
+
+
+    it('should not be able to overwrite an existing public build', done => {
       const regexPrefix9 = `^PR: ${pr9} \\| SHA: ${sha9} \\| File:`;
       const idxContentRegex9 = new RegExp(`${regexPrefix9} \\/index\\.html$`);
       const barContentRegex9 = new RegExp(`${regexPrefix9} \\/foo\\/bar\\.js$`);
@@ -81,4 +177,24 @@ h.runForAllSupportedSchemes((scheme, port) => describe(`integration (on ${scheme
         then(done);
     });
 
+
+    it('should not be able to overwrite an existing hidden build', done => {
+      const regexPrefix9 = `^PR: ${pr9} \\| SHA: ${sha9} \\| File:`;
+      const idxContentRegex9 = new RegExp(`${regexPrefix9} \\/index\\.html$`);
+      const barContentRegex9 = new RegExp(`${regexPrefix9} \\/foo\\/bar\\.js$`);
+
+      h.createDummyBuild(pr9, sha9, false);
+      h.createDummyArchive(pr9, sha9, archivePath);
+
+      uploadBuild(pr9, sha9, archivePath, 'FAKE_VERIFIED_NOT_TRUSTED').
+        then(h.verifyResponse(409)).
+        then(() => {
+          expect(h.readBuildFile(pr9, sha9, 'index.html', false)).toMatch(idxContentRegex9);
+          expect(h.readBuildFile(pr9, sha9, 'foo/bar.js', false)).toMatch(barContentRegex9);
+        }).
+        then(done);
+    });
+
+  });
+
 }));

aio/aio-builds-setup/dockerbuild/scripts-js/lib/verify-setup/upload-server.e2e.ts

@@ -19,7 +19,8 @@ describe('upload-server (on HTTP)', () => {
   describe(`${host}/create-build/<pr>/<sha>`, () => {
     const authorizationHeader = `--header "Authorization: Token FOO"`;
     const xFileHeader = `--header "X-File: ${h.buildsDir}/snapshot.tar.gz"`;
-    const curl = `curl -iL ${authorizationHeader} ${xFileHeader}`;
+    const defaultHeaders = `${authorizationHeader} ${xFileHeader}`;
+    const curl = (url: string, headers = defaultHeaders) => `curl -iL ${headers} ${url}`;
 
 
     it('should disallow non-GET requests', done => {
@@ -42,8 +43,8 @@ describe('upload-server (on HTTP)', () => {
       const bodyRegex = /^Missing or empty 'AUTHORIZATION' header/;
 
       Promise.all([
-        h.runCmd(`curl -iL ${headers1} ${url}`).then(h.verifyResponse(401, bodyRegex)),
+        h.runCmd(curl(url, headers1)).then(h.verifyResponse(401, bodyRegex)),
-        h.runCmd(`curl -iL  ${headers2} ${url}`).then(h.verifyResponse(401, bodyRegex)),
+        h.runCmd(curl(url, headers2)).then(h.verifyResponse(401, bodyRegex)),
       ]).then(done);
     });
 
@@ -55,14 +56,25 @@ describe('upload-server (on HTTP)', () => {
       const bodyRegex = /^Missing or empty 'X-FILE' header/;
 
       Promise.all([
-        h.runCmd(`curl -iL ${headers1} ${url}`).then(h.verifyResponse(400, bodyRegex)),
+        h.runCmd(curl(url, headers1)).then(h.verifyResponse(400, bodyRegex)),
-        h.runCmd(`curl -iL ${headers2} ${url}`).then(h.verifyResponse(400, bodyRegex)),
+        h.runCmd(curl(url, headers2)).then(h.verifyResponse(400, bodyRegex)),
       ]).then(done);
     });
 
 
+    it('should reject requests for which the PR verification fails', done => {
+      const headers = `--header "Authorization: FAKE_VERIFICATION_ERROR" ${xFileHeader}`;
+      const url = `http://${host}/create-build/${pr}/${sha9}`;
+      const bodyRegex = new RegExp(`Error while verifying upload for PR ${pr}: Test`);
+
+      h.runCmd(curl(url, headers)).
+        then(h.verifyResponse(403, bodyRegex)).
+        then(done);
+    });
+
+
     it('should respond with 404 for unknown paths', done => {
-      const cmdPrefix = `${curl} http://${host}`;
+      const cmdPrefix = curl(`http://${host}`);
 
       Promise.all([
         h.runCmd(`${cmdPrefix}/foo/create-build/${pr}/${sha9}`).then(h.verifyResponse(404)),
@@ -78,7 +90,7 @@ describe('upload-server (on HTTP)', () => {
 
 
     it('should reject PRs with leading zeros', done => {
-      h.runCmd(`${curl} http://${host}/create-build/0${pr}/${sha9}`).
+      h.runCmd(curl(`http://${host}/create-build/0${pr}/${sha9}`)).
         then(h.verifyResponse(404)).
         then(done);
     });
@@ -86,48 +98,49 @@ describe('upload-server (on HTTP)', () => {
 
     it('should accept SHAs with leading zeros (but not trim the zeros)', done => {
       Promise.all([
-        h.runCmd(`${curl} http://${host}/create-build/${pr}/0${sha9}`).then(h.verifyResponse(404)),
+        h.runCmd(curl(`http://${host}/create-build/${pr}/0${sha9}`)).then(h.verifyResponse(404)),
-        h.runCmd(`${curl} http://${host}/create-build/${pr}/${sha9}`).then(h.verifyResponse(500)),
+        h.runCmd(curl(`http://${host}/create-build/${pr}/${sha9}`)).then(h.verifyResponse(500)),
-        h.runCmd(`${curl} http://${host}/create-build/${pr}/${sha0}`).then(h.verifyResponse(500)),
+        h.runCmd(curl(`http://${host}/create-build/${pr}/${sha0}`)).then(h.verifyResponse(500)),
       ]).then(done);
     });
 
 
+    [true, false].forEach(isPublic => describe(`(for ${isPublic ? 'public' : 'hidden'} builds)`, () => {
+      const authorizationHeader2 = isPublic ?
+        authorizationHeader : '--header "Authorization: FAKE_VERIFIED_NOT_TRUSTED"';
+      const cmdPrefix = curl('', `${authorizationHeader2} ${xFileHeader}`);
+
+
       it('should not overwrite existing builds', done => {
-      h.createDummyBuild(pr, sha9);
+        h.createDummyBuild(pr, sha9, isPublic);
-      expect(h.readBuildFile(pr, sha9, 'index.html')).toContain('index.html');
+        expect(h.readBuildFile(pr, sha9, 'index.html', isPublic)).toContain('index.html');
 
-      h.writeBuildFile(pr, sha9, 'index.html', 'My content');
+        h.writeBuildFile(pr, sha9, 'index.html', 'My content', isPublic);
-      expect(h.readBuildFile(pr, sha9, 'index.html')).toBe('My content');
+        expect(h.readBuildFile(pr, sha9, 'index.html', isPublic)).toBe('My content');
 
-      h.runCmd(`${curl} http://${host}/create-build/${pr}/${sha9}`).
+        h.runCmd(`${cmdPrefix} http://${host}/create-build/${pr}/${sha9}`).
           then(h.verifyResponse(409, /^Request to overwrite existing directory/)).
-        then(() => expect(h.readBuildFile(pr, sha9, 'index.html')).toBe('My content')).
+          then(() => expect(h.readBuildFile(pr, sha9, 'index.html', isPublic)).toBe('My content')).
           then(done);
       });
 
 
       it('should delete the PR directory on error (for new PR)', done => {
-      const prDir = path.join(h.buildsDir, pr);
+        h.runCmd(`${cmdPrefix} http://${host}/create-build/${pr}/${sha9}`).
-
-      h.runCmd(`${curl} http://${host}/create-build/${pr}/${sha9}`).
           then(h.verifyResponse(500)).
-        then(() => expect(fs.existsSync(prDir)).toBe(false)).
+          then(() => expect(h.buildExists(pr, '', isPublic)).toBe(false)).
           then(done);
       });
 
 
       it('should only delete the SHA directory on error (for existing PR)', done => {
-      const prDir = path.join(h.buildsDir, pr);
+        h.createDummyBuild(pr, sha0, isPublic);
-      const shaDir = path.join(prDir, sha9);
 
-      h.createDummyBuild(pr, sha0);
+        h.runCmd(`${cmdPrefix} http://${host}/create-build/${pr}/${sha9}`).
-
-      h.runCmd(`${curl} http://${host}/create-build/${pr}/${sha9}`).
           then(h.verifyResponse(500)).
           then(() => {
-          expect(fs.existsSync(shaDir)).toBe(false);
+            expect(h.buildExists(pr, sha9, isPublic)).toBe(false);
-          expect(fs.existsSync(prDir)).toBe(true);
+            expect(h.buildExists(pr, '', isPublic)).toBe(true);
           }).
           then(done);
       });
@@ -135,32 +148,34 @@ describe('upload-server (on HTTP)', () => {
 
       describe('on successful upload', () => {
         const archivePath = path.join(h.buildsDir, 'snapshot.tar.gz');
+        const statusCode = isPublic ? 201 : 202;
         let uploadPromise: Promise<CmdResult>;
 
         beforeEach(() => {
           h.createDummyArchive(pr, sha9, archivePath);
-        uploadPromise = h.runCmd(`${curl} http://${host}/create-build/${pr}/${sha9}`);
+          uploadPromise = h.runCmd(`${cmdPrefix} http://${host}/create-build/${pr}/${sha9}`);
         });
-      afterEach(() => h.deletePrDir(pr));
+        afterEach(() => h.deletePrDir(pr, isPublic));
 
 
-      it('should respond with 201', done => {
+        it(`should respond with ${statusCode}`, done => {
-        uploadPromise.then(h.verifyResponse(201)).then(done);
+          uploadPromise.then(h.verifyResponse(statusCode)).then(done);
         });
 
 
         it('should extract the contents of the uploaded file', done => {
           uploadPromise.
             then(() => {
-            expect(h.readBuildFile(pr, sha9, 'index.html')).toContain(`uploaded/${pr}`);
+              expect(h.readBuildFile(pr, sha9, 'index.html', isPublic)).toContain(`uploaded/${pr}`);
-            expect(h.readBuildFile(pr, sha9, 'foo/bar.js')).toContain(`uploaded/${pr}`);
+              expect(h.readBuildFile(pr, sha9, 'foo/bar.js', isPublic)).toContain(`uploaded/${pr}`);
             }).
             then(done);
         });
 
 
         it(`should create files/directories owned by '${h.wwwUser}'`, done => {
-        const shaDir = path.join(h.buildsDir, pr, sha9);
+          const prDir = h.getPrDir(pr, isPublic);
+          const shaDir = path.join(prDir, sha9);
           const idxPath = path.join(shaDir, 'index.html');
           const barPath = path.join(shaDir, 'foo', 'bar.js');
 
@@ -188,7 +203,8 @@ describe('upload-server (on HTTP)', () => {
 
 
         it('should make the build directory non-writable', done => {
-        const shaDir = path.join(h.buildsDir, pr, sha9);
+          const prDir = h.getPrDir(pr, isPublic);
+          const shaDir = path.join(prDir, sha9);
           const idxPath = path.join(shaDir, 'index.html');
           const barPath = path.join(shaDir, 'foo', 'bar.js');
 
@@ -210,6 +226,81 @@ describe('upload-server (on HTTP)', () => {
 
       });
 
+
+      describe('when the PR\'s visibility has changed', () => {
+        const archivePath = path.join(h.buildsDir, 'snapshot.tar.gz');
+        const statusCode = isPublic ? 201 : 202;
+
+        const checkPrVisibility = (isPublic2: boolean) => {
+          expect(h.buildExists(pr, '', isPublic2)).toBe(true);
+          expect(h.buildExists(pr, '', !isPublic2)).toBe(false);
+          expect(h.buildExists(pr, sha0, isPublic2)).toBe(true);
+          expect(h.buildExists(pr, sha0, !isPublic2)).toBe(false);
+        };
+        const uploadBuild = (sha: string) => h.runCmd(`${cmdPrefix} http://${host}/create-build/${pr}/${sha}`);
+
+        beforeEach(() => {
+          h.createDummyBuild(pr, sha0, !isPublic);
+          h.createDummyArchive(pr, sha9, archivePath);
+          checkPrVisibility(!isPublic);
+        });
+        afterEach(() => h.deletePrDir(pr, isPublic));
+
+
+        it('should update the PR\'s visibility', done => {
+          uploadBuild(sha9).
+            then(h.verifyResponse(statusCode)).
+            then(() => {
+              checkPrVisibility(isPublic);
+              expect(h.buildExists(pr, sha9, isPublic)).toBe(true);
+              expect(h.readBuildFile(pr, sha9, 'index.html', isPublic)).toContain(`uploaded/${pr}`);
+              expect(h.readBuildFile(pr, sha9, 'index.html', isPublic)).toContain(sha9);
+            }).
+            then(done);
+        });
+
+
+        it('should not overwrite existing builds (but keep the updated visibility)', done => {
+          expect(h.buildExists(pr, sha0, isPublic)).toBe(false);
+
+          uploadBuild(sha0).
+            then(h.verifyResponse(409, /^Request to overwrite existing directory/)).
+            then(() => {
+              checkPrVisibility(isPublic);
+              expect(h.readBuildFile(pr, sha0, 'index.html', isPublic)).toContain(pr);
+              expect(h.readBuildFile(pr, sha0, 'index.html', isPublic)).not.toContain(`uploaded/${pr}`);
+              expect(h.readBuildFile(pr, sha0, 'index.html', isPublic)).toContain(sha0);
+              expect(h.readBuildFile(pr, sha0, 'index.html', isPublic)).not.toContain(sha9);
+            }).
+            then(done);
+        });
+
+
+        it('should reject the request if it fails to update the PR\'s visibility', done => {
+          // One way to cause an error is to have both a public and a hidden directory for the sme PR.
+          h.createDummyBuild(pr, sha0, isPublic);
+
+          expect(h.buildExists(pr, sha0, isPublic)).toBe(true);
+          expect(h.buildExists(pr, sha0, !isPublic)).toBe(true);
+
+          const errorRegex = new RegExp(`^Request to move '${h.getPrDir(pr, !isPublic)}' ` +
+                                        `to existing directory '${h.getPrDir(pr, isPublic)}'.`);
+
+          uploadBuild(sha9).
+            then(h.verifyResponse(409, errorRegex)).
+            then(() => {
+              expect(h.buildExists(pr, sha0, isPublic)).toBe(true);
+              expect(h.buildExists(pr, sha0, !isPublic)).toBe(true);
+              expect(h.buildExists(pr, sha9, isPublic)).toBe(false);
+              expect(h.buildExists(pr, sha9, !isPublic)).toBe(false);
+            }).
+            then(done);
+        });
+
+      });
+
+    }));
+
   });