Fix playwright test flakiness (#1495)

* Fix playwright test flakiness

During the saved search playwright tests, we kill and reopen the ports needed for the database (as well as all other ports).

We do this because beforehand, we did not port forward the ports for the database and only the ports needed to connect to the app (the backend already had access to the database internally)

But instead of doing that, we now port forward the database ports. Then we have a new helper makefile target to check if the port is live.

We use `nc` instead of curl because it did not work with grpc. The devcontainer Dockerfile has been updated to install that dependency (devs will need to rebuild their devcontainer)

Hopefully, this will reduce the times when the port fails to forward again on subsequent tries since we never have to forward it again.

One thing to note: Now developers will need to run make port-forward-manual first before dev_fake_data and other commands after running start-local

* revert limit for datastore

* further increase spanner limit

* dev_fake_data already calls check-local-ports

Author: jcscottiii

.dev/auth/manifests/pod.yaml

@@ -28,6 +28,10 @@ spec:
           name: auth-port
         - containerPort: 9100
           name: auth-ui-port
+      readinessProbe:
+        tcpSocket:
+          port: 9099
+        initialDelaySeconds: 15
       startupProbe:
         httpGet:
           port: auth-ui-port

.dev/datastore/manifests/pod.yaml

@@ -33,3 +33,7 @@ spec:
         requests:
           cpu: 100m
           memory: 128Mi
+      readinessProbe:
+        tcpSocket:
+          port: 8086
+        initialDelaySeconds: 10

.dev/spanner/manifests/pod.yaml

@@ -28,10 +28,14 @@ spec:
           name: grpc-port
         - containerPort: 9020
           name: rest-port
+      readinessProbe:
+        tcpSocket:
+          port: 9010
+        initialDelaySeconds: 10
       resources:
         limits:
           cpu: '2'
-          memory: '3.5Gi'
+          memory: '5.5Gi'
         requests:
           cpu: '300m'
           memory: '256Mi'

.dev/valkey/manifests/pod.yaml

@@ -26,6 +26,9 @@ spec:
       ports:
         - containerPort: 6379
           name: valkey-port
+      readinessProbe:
+        tcpSocket:
+          port: 6379
       resources:
         limits:
           cpu: 250m

.devcontainer/Dockerfile

@@ -35,6 +35,9 @@ RUN echo "alias grun='java org.antlr.v4.gui.TestRig'" >> ~/.bashrc
 
 ENV CLASSPATH=.:/usr/local/lib/antlr-${ANTLR4_VERSION}-complete.jar
 
+# Install netcat (nc)
+RUN apt-get update && apt-get install -y netcat-traditional
+
 # Switch to the default user and append to PATH
 USER vscode
 

Makefile

@@ -62,11 +62,10 @@ deploy-local: configure-skaffold
 delete-local:
 	skaffold delete $(SKAFFOLD_FLAGS) || true
 
-# TODO: In the future, we should install netcat
 define wait_for_port
     @echo "Waiting for $(2) on port $(1) to respond..."
     @for i in $$(seq 1 5); do \
-        if curl -s -f -o /dev/null -m 2 http://localhost:$(1)$(3); then \
+        if nc -zvw 2 localhost $(1); then \
             echo "$(2) on port $(1) is responding."; \
             exit 0; \
         fi; \
@@ -80,30 +79,42 @@ define wait_for_port
     done
 endef
 
+
+check-local-ports:
+	$(call wait_for_port,5555,frontend)
+	$(call wait_for_port,8080,backend)
+	$(call wait_for_port,9099,auth-main)
+	$(call wait_for_port,9100,auth-aux)
+	$(call wait_for_port,9010,spanner)
+	$(call wait_for_port,8086,datastore)
+
 port-forward-manual: port-forward-terminate
 	kubectl wait --for=condition=ready pod/frontend
 	kubectl wait --for=condition=ready pod/backend
 	kubectl wait --for=condition=ready pod/auth
+	kubectl wait --for=condition=ready pod/datastore
+	kubectl wait --for=condition=ready pod/spanner
 	kubectl port-forward --address 127.0.0.1 pod/frontend 5555:5555 2>&1 >/dev/null &
 	kubectl port-forward --address 127.0.0.1 pod/backend 8080:8080 2>&1 >/dev/null &
 	kubectl port-forward --address 127.0.0.1 pod/auth 9099:9099 2>&1 >/dev/null &
 	kubectl port-forward --address 127.0.0.1 pod/auth 9100:9100 2>&1 >/dev/null &
-	$(call wait_for_port,5555,frontend,/)
-	$(call wait_for_port,8080,backend,/v1/features?page_size=1)
-	$(call wait_for_port,9099,auth-main,/)
-	$(call wait_for_port,9100,auth-aux,/)
+	kubectl port-forward --address 127.0.0.1 pod/spanner 9010:9010 2>&1 >/dev/null &
+	kubectl port-forward --address 127.0.0.1 pod/datastore 8086:8086 2>&1 >/dev/null &
+	make check-local-ports
 
 port-forward-terminate:
 	fuser -k 5555/tcp || true
 	fuser -k 8080/tcp || true
 	fuser -k 9099/tcp || true
 	fuser -k 9100/tcp || true
+	fuser -k 9010/tcp || true
+	fuser -k 8086/tcp || true
 
 # Prerequisite target to start minikube if necessary
 minikube-running:
 		# Check if minikube is running using a shell command
 		@if ! minikube status -p "$${MINIKUBE_PROFILE}" | grep -q "Running"; then \
-				minikube start -p "$${MINIKUBE_PROFILE}" --cni calico --disk-size=10gb --cpus=2 --memory=4096m; \
+				minikube start -p "$${MINIKUBE_PROFILE}" --cni calico --disk-size=10gb --cpus=2 --memory=6g; \
 		fi
 minikube-clean-restart: minikube-delete minikube-running
 minikube-delete:
@@ -313,7 +324,7 @@ license-fix:
 # fresh-env-for-playwright prerequisite. If unset, the fresh environment will be created.
 SKIP_FRESH_ENV ?=
 
-fresh-env-for-playwright: $(if $(SKIP_FRESH_ENV),,playwright-install delete-local build deploy-local dev_fake_users dev_fake_data port-forward-manual)
+fresh-env-for-playwright: $(if $(SKIP_FRESH_ENV),,playwright-install delete-local build deploy-local port-forward-manual dev_fake_users dev_fake_data)
 
 playwright-install:
 	npx playwright install --with-deps
@@ -413,26 +424,14 @@ dev_fake_users: build
 	fuser -k 9099/tcp || true
 	kubectl port-forward --address 127.0.0.1 pod/auth 9099:9099 2>&1 >/dev/null &
 	go run util/cmd/load_test_users/main.go -project=local
-dev_fake_data: build is_local_migration_ready
-	fuser -k 9010/tcp || true
-	kubectl wait --for=condition=ready pod/spanner
-	kubectl port-forward --address 127.0.0.1 pod/spanner 9010:9010 2>&1 >/dev/null &
-	fuser -k 8086/tcp || true
-	kubectl wait --for=condition=ready pod/datastore
-	kubectl port-forward --address 127.0.0.1 pod/datastore 8086:8086 2>&1 >/dev/null &
-	fuser -k 9099/tcp || true
-	kubectl wait --for=condition=ready pod/auth
-	kubectl port-forward --address 127.0.0.1 pod/auth 9099:9099 2>&1 >/dev/null &
+dev_fake_data: build is_local_migration_ready check-local-ports
 	SPANNER_EMULATOR_HOST=localhost:9010 DATASTORE_EMULATOR_HOST=localhost:8086 FIREBASE_AUTH_EMULATOR_HOST=localhost:9099 \
 		go run ./util/cmd/load_fake_data/main.go \
 			-spanner_project=local \
 			-spanner_instance=local \
 			-spanner_database=local \
 			-datastore_project=local \
 			$(LOAD_FAKE_DATA_FLAGS)
-	fuser -k 9099/tcp || true
-	fuser -k 9010/tcp || true
-	fuser -k 8086/tcp || true
 is_local_migration_ready:
 	kubectl wait --for=condition=ready --timeout=300s pod/spanner
 	@MAX_RETRIES=5; SLEEP_INTERVAL=5 ; \

backend/manifests/pod.yaml

@@ -26,6 +26,9 @@ spec:
       ports:
         - containerPort: 8080
           name: http-backend
+      readinessProbe:
+        tcpSocket:
+          port: 8080
       env:
         - name: SPANNER_DATABASE
           value: 'local'

docs/testing.md

@@ -61,10 +61,10 @@ Developers may still need to inspect and manually fix the error.
 ### Note about running Playwright Makefile Targets
 
 If you have a running local development environment
-(established via `make start-local`) with the necessary fake users (`make dev_fake_users`),
-fake data (`make dev_fake_data`), and port forwarding
-(`make port-forward-manual`), you can expedite Playwright testing by skipping
-the environment setup.
+(established via `make start-local`) with the necessary port forwarding
+(`make port-forward-manual`), fake users (`make dev_fake_users`) and
+fake data (`make dev_fake_data`), you can expedite Playwright testing by
+skipping the environment setup.
 
 To do this, prefix your `make` command with `SKIP_FRESH_ENV=1`. This is
 particularly useful for rapid iteration, as `make start-local` leverages

e2e/tests/utils.ts

@@ -122,19 +122,12 @@ export async function resetUserData() {
   const projectRootDir = path.resolve(__dirname, '../..');
 
   try {
-    const cmd1 = `make dev_fake_data -o build -o is_local_migration_ready LOAD_FAKE_DATA_FLAGS='-scope=user -reset'`;
+    const cmd = `make dev_fake_data -o build -o is_local_migration_ready LOAD_FAKE_DATA_FLAGS='-scope=user -reset'`;
 
-    console.log(`Executing command: ${cmd1} in ${projectRootDir}`);
-    execSync(cmd1, {cwd: projectRootDir, stdio: 'inherit'});
+    console.log(`Executing command: ${cmd} in ${projectRootDir}`);
+    execSync(cmd, {cwd: projectRootDir, stdio: 'inherit'});
 
     console.log('Reset command finished successfully.');
-
-    const cmd2 = 'make port-forward-manual';
-    console.log(`Executing command: ${cmd2} in ${projectRootDir}`);
-    execSync(cmd2, {
-      cwd: projectRootDir,
-      stdio: 'inherit',
-    });
   } catch (error) {
     console.error('Error reset command (make dev_fake_data):', error);
     throw new Error('Reset command finished, halting tests.');

frontend/manifests/pod.yaml

@@ -29,6 +29,9 @@ spec:
       ports:
         - containerPort: 5555
           name: http-frontend
+      readinessProbe:
+        tcpSocket:
+          port: 5555
       env:
         - name: API_URL
           value: http://localhost:8080