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

Resolves #207 - Adding automated tests to CI #213

Merged
merged 13 commits into from
May 29, 2024
Merged

Resolves #207 - Adding automated tests to CI #213

merged 13 commits into from
May 29, 2024

Conversation

alexcottner
Copy link
Contributor

@alexcottner alexcottner commented May 15, 2024

Resolves #207 - adding automated tests to CI pipeline.

This is adding selenium tests that we can run to have confidence in our changes before merging and/or running against a dev firefox branch.

ToDo:

  • Add a couple more tests
  • Integrate into github actions
  • Clean up code (maybe add prettier?)
  • Updated readme with notes on running automated tests

Notes:

  • This works well with selenium-webdriver, although we have migrated away from that in some of our other projects
  • Tried to do this with playwright at first, but extension testing for firefox appears unsupported at this time.
  • Tried to do this with puppeteer by connecting to the web-ext browser as well, but it kept hanging. The dom loaded events seemed to not work quite right.
  • Expecting some potentially large feedback from this one, so let it rip.

@alexcottner alexcottner changed the title WIP - Resolves #207 - Adding automated tests to CI Resolves #207 - Adding automated tests to CI May 17, 2024
@alexcottner alexcottner marked this pull request as ready for review May 20, 2024 18:47
README.md Show resolved Hide resolved
tests/selenium.spec.js Show resolved Hide resolved

// clear all data
await clickByCss("#clear-all-data");
await driver.sleep(1000); // wait for all collections to be cleared
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we wait for a certain selector instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem here is it marks the rows as busy/ready one at a time, so the waitForLoad call doesn't work reliably as it's a race condition. Maybe we could use DOMAttrModified events and give ourselves a debounce so we wait 200ms after the last busy is removed? 🤔

Copy link
Contributor Author

@alexcottner alexcottner May 23, 2024

Choose a reason for hiding this comment

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

Switched to a debounce timeout that relies on a MutationObserver. It's kinda ugly. It has to inject an element to work because I don't see a way to pipe those events out of the browser (there appears to be a chrome option but no firefox option).

But, it seems to work perfectly. I tried to make the code as readable as possible.

@alexcottner alexcottner requested a review from leplatrem May 23, 2024 15:40
@leplatrem leplatrem merged commit 18f2ee3 into master May 29, 2024
8 checks passed
@leplatrem leplatrem deleted the selenium branch May 29, 2024 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DevTools - Add unit tests to github actions
2 participants