Skip to content

Fix playwright test flakiness#1495

Merged
jcscottiii merged 4 commits intomainfrom
jcscottiii/fix-pwtests-flakiness
May 16, 2025
Merged

Fix playwright test flakiness#1495
jcscottiii merged 4 commits intomainfrom
jcscottiii/fix-pwtests-flakiness

Conversation

@jcscottiii
Copy link
Copy Markdown
Collaborator

@jcscottiii jcscottiii commented May 15, 2025

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 forwarded 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 during make port-forward-manual. To help with that, I modified the helper makefile target to check if the port is live. We use nc instead of curl because it did not work with grpc (which the spanner emulator uses). The devcontainer Dockerfile has been updated to install that dependency (devs will need to rebuild their devcontainer)

I also started to add readinessProbes for some of the pods. And for the database and auth pods, I modified the initial delay before checking for readiness since sometimes those take some time.

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

Resources:

  • I allocated more resources to the local kubernetes cluster
    • With this, I increased the limits for some pods.

A few things to note:

  • Now developers will need to run make port-forward-manual first before dev_fake_data and other commands after running start-local
  • As mentioned earlier, this change requires developers to rebuild their devcontainer

@jcscottiii jcscottiii force-pushed the jcscottiii/fix-pwtests-flakiness branch 6 times, most recently from 2a91627 to d10a68b Compare May 16, 2025 13:55
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
@jcscottiii jcscottiii force-pushed the jcscottiii/fix-pwtests-flakiness branch from d10a68b to 12f5bce Compare May 16, 2025 13:56
Copy link
Copy Markdown
Collaborator

@DanielRyanSmith DanielRyanSmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for looking into this. 🙂

Comment thread .dev/spanner/manifests/pod.yaml Outdated
@jcscottiii jcscottiii added this pull request to the merge queue May 16, 2025
Merged via the queue into main with commit 6965d9b May 16, 2025
7 checks passed
@jcscottiii jcscottiii deleted the jcscottiii/fix-pwtests-flakiness branch May 16, 2025 16:40
@jcscottiii jcscottiii mentioned this pull request May 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants