From b6304c7a0293b5bb4cc966cf895ad3ebb388a79f Mon Sep 17 00:00:00 2001
From: Victoria Dye <vdye@github.com>
Date: Wed, 10 May 2023 11:36:47 -0700
Subject: [PATCH] test: verify that bundle server started

Update the 'bundleServer.startWebServer()' method to wait for indication
that the web server has successfully started (the "Server is running at
address..." message) before returning successfully. If the process stops
before that, or a timeout of 2s is reached, an error is thrown.

However, because the "Server is running..." printout happens in parallel
with the setup done in 'ListenAndServe[TLS]()', it is possible that the
message is printed and immediately followed with an exit due to a server
startup error. To mitigate that, add a 0.1s pause before printing the
message and, in the tests, only set the status to 'ok' if the 'close' event
has not been triggered by the time the startup message is processed.

Signed-off-by: Victoria Dye <vdye@github.com>
---
 cmd/git-bundle-web-server/bundle-server.go    | 10 +++++++
 test/shared/classes/bundleServer.ts           | 30 +++++++++++++++++--
 .../features/step_definitions/bundleServer.ts |  2 +-
 3 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/cmd/git-bundle-web-server/bundle-server.go b/cmd/git-bundle-web-server/bundle-server.go
index b495f1a..8e03eed 100644
--- a/cmd/git-bundle-web-server/bundle-server.go
+++ b/cmd/git-bundle-web-server/bundle-server.go
@@ -185,6 +185,16 @@ func (b *bundleWebServer) StartServerAsync(ctx context.Context) {
 		}
 	}(ctx)
 
+	// Wait 0.1s before reporting that the server is started in case
+	// 'listenAndServeFunc' exits immediately.
+	//
+	// It's a hack, but a necessary one because 'ListenAndServe[TLS]()' doesn't
+	// have any mechanism of notifying if it starts successfully, only that it
+	// fails. We could get around that by copying/reimplementing those functions
+	// with a print statement inserted at the right place, but that's way more
+	// cumbersome than just adding a delay here (see:
+	// https://stackoverflow.com/questions/53332667/how-to-notify-when-http-server-starts-successfully).
+	time.Sleep(time.Millisecond * 100)
 	fmt.Println("Server is running at address " + b.server.Addr)
 }
 
diff --git a/test/shared/classes/bundleServer.ts b/test/shared/classes/bundleServer.ts
index 90aa36b..94ccef0 100644
--- a/test/shared/classes/bundleServer.ts
+++ b/test/shared/classes/bundleServer.ts
@@ -21,12 +21,38 @@ export class BundleServer {
     this.bundleWebServerCmd = bundleWebServerCmd
   }
 
-  startWebServer(port: number): void {
+  async startWebServer(port: number): Promise<void> {
     if (this.webServerProcess) {
       throw new Error("Tried to start web server, but web server is already running")
     }
-    this.webServerProcess = child_process.spawn(this.bundleWebServerCmd, ["--port", String(port)])
+    const webServerProcess = child_process.spawn(this.bundleWebServerCmd, ["--port", String(port)])
+    this.webServerProcess = webServerProcess
     this.bundleUriBase = `http://localhost:${port}/`
+
+    // Now, ensure the server is running
+    var timer: NodeJS.Timeout | undefined
+    var ok: boolean | undefined
+
+    await Promise.race([
+      new Promise<void>(resolve => webServerProcess.stdout.on('data', (data: string) => {
+        if (data.includes("Server is running at address") && ok === undefined) {
+          ok = true
+          resolve() // server is running
+        }
+      })),
+      new Promise<void>(resolve => webServerProcess.on('close', () => {
+        ok = false
+        resolve() // program failed to start/exited early
+      })),
+      new Promise(resolve => timer = setTimeout(resolve, 2000)) // fallback timeout
+    ])
+
+    // If it's still running, clear the timeout so it doesn't delay shutdown
+    clearTimeout(timer)
+
+    if (!ok) {
+      throw new Error('Failed to start web server')
+    }
   }
 
   init(remote: RemoteRepo, routePrefix: string, route: string = ""): child_process.SpawnSyncReturns<Buffer> {
diff --git a/test/shared/features/step_definitions/bundleServer.ts b/test/shared/features/step_definitions/bundleServer.ts
index 5d1cc26..220e5b8 100644
--- a/test/shared/features/step_definitions/bundleServer.ts
+++ b/test/shared/features/step_definitions/bundleServer.ts
@@ -2,5 +2,5 @@ import { Given } from '@cucumber/cucumber'
 import { BundleServerWorldBase } from '../../support/world'
 
 Given('the bundle web server was started at port {int}', async function (this: BundleServerWorldBase, port: number) {
-  this.bundleServer.startWebServer(port)
+  await this.bundleServer.startWebServer(port)
 })