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

BREAKING: Refactor whole codebase #145

Open
wants to merge 35 commits into
base: main
Choose a base branch
from

Conversation

ebebbington
Copy link
Member

@ebebbington ebebbington commented Apr 30, 2024

General

  • Added a timeout for sending messages to the websocket. Should a message be sent a response not received for30 seconds, the process will close and an error will be thrown

buildFor

  • Changed to Client.create()
  • Removed browser parameter from the options, it only uses chrome now

Browser/Client

.page()

  • removed

This is part of the bigger picture to remove this page management we felt wasn't needed

.closeAllPagesExcept()

  • Removed

This is part of the bigger picture to remove this page management we felt wasn't needed

Elements

  • Removed, please see what you should be using instead below

.file()/.files()

  • Use page.setInputFiles({ selector: 'string', files: array<string> }) instead

.value()

  • use .evaluate() instead

.takeScreenshot()

  • use page.screenshot({ element: 'string' }) instead

.click()

  • use page.evaluate('document.querySelector("button").click()'); instead
  • Utilise waitUntilNetworkIdle() if clicking triggers any form of network request
  • Use page.newPageClick() when clicking something that will open a new tab

Page

.querySelector()

  • Removed, as the element class was removed, this method was no longer used

.close()

  • Removed, no longer needed and covered by browser.close()

.expectWaitForRequest()

  • Removed, felt it wasn't needed and can be covered by waitUntilNetworkIdle()

.location()

  • location parameter is now required, if you wish to get the current url, you can use .evaluate(window.location.href) instead
  • Added extra error handling

.assertNoConsoleErrors() --> .consoleErrors()

  • renamed method
  • It now returns an array of strings which you can assert if you wish

.takeScreenshot()

  • Renamed to .screenshot()
  • Removed path parameter
  • Now returns a unint8array

Idea was to give the responsibility of handling/saving the file to the user, so you can save it if you wish

.expectDialog

  • Removed, no longer needed, if actioning to trigger a dialog (e.g., calling .evaluate()) then do not await evaluate

@ebebbington ebebbington changed the title Replace assertNoConsoleErrors with consoleErrors BREAKING: Refactor whole codebase May 2, 2024
@ebebbington ebebbington linked an issue May 15, 2024 that may be closed by this pull request
@ebebbington ebebbington mentioned this pull request May 15, 2024
@ebebbington ebebbington marked this pull request as ready for review May 15, 2024 22:23
@ebebbington ebebbington requested review from crookse and Guergeiro May 15, 2024 22:23
@ebebbington
Copy link
Member Author

Big change log i need to eventually work up.. but in a nutshell:

(note: idea is to just be simple man... a lightweight way to just run a subprocess to get some browser tests done)

  • Removed firefox support
  • Removed remote support (kind of, you now instead use connect())
  • Removed a bunch of api methods that are just covered by evaluate
  • The original idea was to be 1:1 with the browser apis, but it just wasn't possible, eg cookie, takeScreenshot, so that idea was scrapped

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Listen for script exit, and close processes
1 participant