Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tidy up and reapply our stability fixes / logging / etc to the newer release #20

Open
4 of 35 tasks
acoulton opened this issue Oct 1, 2024 · 0 comments
Open
4 of 35 tasks

Comments

@acoulton
Copy link
Member

acoulton commented Oct 1, 2024

CODING STYLE

  • ccc5ac5 Switch array unpacking to use new php 7.1 syntax (needs to go in standalone as it will conflict with lots of other things)

LOGGING

Maybe just for our fork? Although it could generally be useful to have...

  • f950bda Add quick and dirty (and nasty) Chrome debug logger
  • d84c6ae Include failure to reset session in the debug log
  • fe5d95b Record what causes page ready state to change
  • a2d0749 Capture page_ready state and current scenario in chrome logs
  • f3492b0 Improve debug logging on error attempting to wipe basket
  • 30cfa31 Re-run failed behat scenarios to see if they pass on a retry
  • 0694174 Make the log file path configurable
  • 4b10385 Fix ::initialise should be static

NEEDED - JS Dialog handling

Our removal of processResponse relied on our custom JS dialog handling. Need to keep it as-was and make sure instead that wait, waitfor, command execution etc work as-expected when a dialog is open.

  • 9c7ae5b Don't return a value from processResponse

NEEDED - Page load tracking / timeout handling

Needs to be consolidated / cleaned up and made to work with the driver as-is and what we know now about the cases in which the loads were not completing.

  • 93070bc Refactor how timeouts & exceptions are handled on websocket
  • c4e6335 Apply a default 90 second timeout to all waitFor if not otherwise specified
  • ce60162 Remove duplications between waitForDom & waitForLoad
  • be26c69 Surely Page.loadEventFired should mark the page ready?
  • 2a1d451 Attempt to simplify & improve how we wait for pages to load
  • 5cda60d Fix detecting "page load" after internal navigation
  • 0af4e15 Apply page load timeout from when page starts loading
  • d09e430 Don't sleep when waiting for a possible navigation
  • 4cd8191 Use Browser.getVersion instead of Runtime.evaluate to check if need to load
  • 7e47235 Document pending things that might not quite work
  • f953f11 Fix deadlock waiting for pageload if it's a download

MAYBE SOME OF, NOT THE REST - Opt out of network-level / pending_requests tracking

This would remove some driver features, and was potentially not the best way to handle it as it's complicating page-load tracking slightly I think. Review in parallel with the page-load tracking now we have a better idea of what's needed there.

  • 596f43a Attempt to cleanup pending_requests implementation
  • c7df3f3 What happens if we opt out of Network events?

NEEDED - Crash recovery

  • c5b6749 Attempt to handle chrome crashes properly

MAYBE - get the current URL without waiting for a navigation to complete

Note might break some usecases if it no longer waits for a nav to complete? Although in practice historically the driver would usually block during a nav anyway, so this will only ever have been the current URL?

  • 9763728 Don't use the websocket for getting current URL

MAYBE - force reset to about:blank in a new window if the browser crashes

  • 1d1454d Attempt to forcibly close & restart browser if it hangs
  • 9e84d0e Attempt to improve nav to about:blank after scenarios and stop/start
  • 60ba29d Only need to navigate back to about:blank if scenario has navigated

DEBUGGING (maybe not required?)

  • 5bea4d5 Hackily log if the underlying websocket connects more than once
  • 3e9d03b (origin/spike-experimental-reliability-enhancements) Fix ConnLoggingWebsocketClient for new public client connect

NEEDED - new feature, clear localstorage

  • 020386e Natively flush Chrome localstorage for the domain

Small connection fixes

All proposed in https://gitlab.com/behat-chrome/chrome-mink-driver/-/merge_requests/188

  • 9713f37 Treat a null websocket return as an exception
  • dc3631e Only reset request headers after scenario if they were set
  • fec0010 Don't take animation events, they aren't actually used.
  • 59bfbde Remove handler for Security.certificateError

Not required

  • 9a5881a See if we can catch the exceptions that cause the behat to crash outright not needed, that was a process termination thing
  • c8bf848 Include the nature of the failure in the StreamReadException message StreamReadException is no longer thrown
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

No branches or pull requests

1 participant