Skip to content

Add a retry for setting the cookie in webview tests.#3103

Merged
mhsmith merged 2 commits into
beeware:mainfrom
freakboy3742:cookie-retry
Jan 13, 2025
Merged

Add a retry for setting the cookie in webview tests.#3103
mhsmith merged 2 commits into
beeware:mainfrom
freakboy3742:cookie-retry

Conversation

@freakboy3742

Copy link
Copy Markdown
Member

#3068 added an API to retrieve cookies from WebView; however, the test for this feature has been flaky on iOS and macOS. This appears to be due to a race condition in the testbed - the on_load signal isn't a 100% reliable mechanism to prove that the webview is fully initialised and ready to accept Javascript calls.

You can overcome this by adding a short delay before setting the cookie. This PR adds a retry strategy to setting the cookie so that the delay is as short as is practical.

PR Checklist:

  • All new features have been tested
  • All new features have been documented
  • I have read the CONTRIBUTING.md file
  • I will abide by the code of conduct

@freakboy3742 freakboy3742 requested a review from mhsmith January 13, 2025 09:36
@mhsmith mhsmith merged commit 58d0d02 into beeware:main Jan 13, 2025
@freakboy3742 freakboy3742 deleted the cookie-retry branch January 13, 2025 22:23
@mhsmith

mhsmith commented Jan 14, 2025

Copy link
Copy Markdown
Member

Still intermittently failing on iOS: https://github.com/beeware/toga/actions/runs/12777096862/job/35617329195

@freakboy3742

Copy link
Copy Markdown
Member Author

That's definitely the same test failing, but the manifestation is quite different.

Most notably, the exception that has surfaced is a TimeoutError - which isn't something that the testbed has raised directly. It's also notable that the cookie has only been retried 3 times (or, at least, the log message has only been printed 3 times) rather than 5.

If this was the same error, I'd expect to see 5 retries, then a "Cookie was not set" exception from L366. This stack trace looks more like the cookie test has passed after 3 tests, but then the test cleanup mechanism has failed for some reason. That's still a bug that needs to be fixed - but it's a different bug, and could be one that has been caused by some other state issue on the test machine. We'll have to see how often this new manifestation occurs.

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