Improve playwright execution to fix flakiness (streamlit#7514)

* Add auto rerun on failed tests

* Change message

* Add report output  and install also for test req

* Install github actions tool for annotations

* Use alternative method to find free port

* Move playwright deps install to test requirements

* Don't upload snapshots directory

* Renaming

* Add extra playwright install step

.github/workflows/playwright.yml

@@ -44,9 +44,11 @@ jobs:
         uses: ./.github/actions/make_init
       - name: Run make develop
         run: make develop
+      - name: Install playwright
+        run: python -m playwright install --with-deps
       - name: Run make protobuf
         run: make protobuf
-      - name: Run make frontend
+      - name: Run make frontend-fast
         run: make frontend-fast
       - name: Run make playwright
         run: make playwright
@@ -61,12 +63,6 @@ jobs:
             git ls-files --others --exclude-standard | grep snapshots;
             exit 1;
           fi
-      - name: Upload snapshots
-        uses: actions/upload-artifact@v3
-        if: always()
-        with:
-          name: playwright_snapshots
-          path: e2e_playwright/__snapshots__
       - name: Upload failed test results
         uses: actions/upload-artifact@v3
         if: always()

Makefile

@@ -114,6 +114,9 @@ python-init:
 	fi;\
 	echo "Running command: pip install $${pip_args[@]}";\
 	pip install $${pip_args[@]};
+	if [ "${INSTALL_TEST_REQS}" = "true" ] ; then\
+		python -m playwright install --with-deps; \
+	fi;\
 
 .PHONY: pylint
 # Verify that our Python files are properly formatted.
@@ -337,10 +340,9 @@ e2etest:
 .PHONY: playwright
 # Run playwright E2E tests.
 playwright:
-	python -m playwright install --with-deps; \
 	cd e2e_playwright; \
 	rm -rf ./test-results; \
-	pytest --browser webkit --browser chromium --browser firefox --video retain-on-failure --screenshot only-on-failure --output ./test-results/ -n auto -v
+	pytest --browser webkit --browser chromium --browser firefox --video retain-on-failure --screenshot only-on-failure --output ./test-results/ -n auto --reruns 1 --reruns-delay 1 --rerun-except "Missing snapshot" -r aR -v
 
 .PHONY: loc
 # Count the number of lines of code in the project.

e2e_playwright/conftest.py

@@ -18,7 +18,6 @@
 """
 from __future__ import annotations
 
-import contextlib
 import os
 import re
 import shlex
@@ -29,6 +28,7 @@
 import time
 from io import BytesIO
 from pathlib import Path
+from random import randint
 from tempfile import TemporaryFile
 from types import ModuleType
 from typing import Any, Generator, List, Literal, Protocol
@@ -102,12 +102,36 @@ def resolve_test_to_script(test_module: ModuleType) -> str:
     return test_module.__file__.replace("_test.py", ".py")
 
 
-def find_available_port(host: str = "localhost") -> int:
+def is_port_available(port: int, host: str) -> bool:
+    """Check if a port is available on the given host."""
+    with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock:
+        return sock.connect_ex((host, port)) != 0
+
+
+def find_available_port(
+    min_port: int = 10000,
+    max_port: int = 65535,
+    max_tries: int = 50,
+    host: str = "localhost",
+) -> int:
     """Find an available port on the given host."""
-    with contextlib.closing(socket.socket(socket.AF_INET, socket.SOCK_STREAM)) as s:
-        s.bind((host, 0))  # 0 means that the OS chooses a random port
-        s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
-        return int(s.getsockname()[1])  # [1] contains the randomly selected port number
+    for _ in range(max_tries):
+        selected_port = randint(min_port, max_port)
+        if is_port_available(selected_port, host):
+            return selected_port
+    raise RuntimeError("Unable to find an available port.")
+
+
+# TODO(lukasmasuch): This was the previous method to rely on the OS to find a free port.
+# but when running the tests in parallel, it can happen that the OS assigns the same port
+# to multiple tests. This is why we now use the find_available_port method above.
+
+# def find_available_port(host: str = "localhost") -> int:
+#     """Find an available port on the given host."""
+#     with contextlib.closing(socket.socket(socket.AF_INET, socket.SOCK_STREAM)) as s:
+#         s.bind((host, 0))  # 0 means that the OS chooses a random port
+#         s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
+#         return int(s.getsockname()[1])  # [1] contains the randomly selected port number
 
 
 def is_app_server_running(port: int, host: str = "localhost") -> bool:
@@ -419,4 +443,4 @@ def compare(
     yield compare
 
     if test_failure_messages:
-        pytest.fail("Snapshot test failed \n" + "\n".join(test_failure_messages))
+        pytest.fail("Missing snapshots: \n" + "\n".join(test_failure_messages))

lib/test-requirements.txt

@@ -29,6 +29,8 @@ testfixtures
 pytest-playwright>=0.1.2
 pixelmatch>=0.3.0
 pytest-xdist
+pytest-rerunfailures
+pytest-github-actions-annotate-failures
 
 mypy-protobuf>=3.2, <3.4