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

more e2e tests for batteries #1028

Draft
wants to merge 25 commits into
base: master
Choose a base branch
from
Draft

Conversation

riccardobl
Copy link
Contributor

@riccardobl riccardobl commented Jun 10, 2022

This PR extends e2e tests to detect when batteries break.
I provided my own test links for now, they should be eventually replaced by something that is under the control of the core team.

Current coverage:

  • Twitter
  • Twitch
  • Instagram *
  • Github repo description
  • Github repo readme
  • Github user bio
  • Github user readme
  • Github repo to user bio fallback
  • Github repo to user readme fallback
  • Medium Post
  • Medium Author Page
  • Medium Author About Page
  • Monetization
  • Peertube
  • Podcastindex
  • Vimeo
  • Youtube Channel
  • Youtube Video
  • Reddit

*it seems IG hates any form of automated interaction, so i had to add some random delays to make it somewhat reliable. Also, since it limits the interactions for anonymous users, the script tries to login with the provided ALBY_E2E_INSTAGRAM_USERNAME and ALBY_E2E_INSTAGRAM_PASSWORD
env variables, if they are set. It is better to use a throw away account here, since it might trigger some anti-bot filter , and it might be against their ToS (i haven't actually checked...)....

The missing tests are mostly due to the fact that i need to setup an account for each of them, so if you happen to know the public link to an account that is already configured (or you want to make a new one...), feel free to share it here and i will look into adding it to the missing tests.

Note: i merged my other PRs #1019 , #1017 , #1014 in this PR to write the tests for them. I will rebase the code to whatever the current master is before the merge. Only commits
80d279f

fd2a652

bdc61f7

are actually relevant to this PR

@riccardobl riccardobl changed the title E2ebattery E2E tests for Batteries Jun 10, 2022
@riccardobl
Copy link
Contributor Author

riccardobl commented Jun 10, 2022

the debug=true hardcoded variables here

let debug = true; // FIXME
if (debug) {

and here
const debug = true; // FIXME
if (debug) {

are going to be replaced with some code that check if the extension is in production or development, once i figure out how to do it

Solved

@riccardobl
Copy link
Contributor Author

I've changed the code to disable the instagram test if ALBY_E2E_INSTAGRAM_USERNAME or ALBY_E2E_INSTAGRAM_PASSWORD are missing.
This is to avoid breaking the CI in PRs where gh secrets are not shareable anyway.

@escapedcat
Copy link
Contributor

This is really cool! I'm a bit worried this is getting a big too big for just one PR.
Would you mind creating one PR with the general and just one battery test. Including some documentation how everything is supposed to work? This would be super helpful for others and we could focus on the general setup first and then move in more tests with follow-up PRs?

@riccardobl
Copy link
Contributor Author

riccardobl commented Jun 13, 2022

I can, but it isn't so big actually, 90% of the commits here are merges of my other PRs (as they add batteries..) maybe we should wait to see if they get rejected or merged first.

And i still haven't figured out why this code timeouts on gh action while it works fine locally

@riccardobl
Copy link
Contributor Author

Thank you again for helping me solve the timeout issue @escapedcat . I moved the general implementation to this PR #1041 and I will refactor this one to extend the test coverage on top of #1041

@riccardobl riccardobl changed the title E2E tests for Batteries more e2e tests for batteries Jun 14, 2022
@im-adithya
Copy link
Member

Oops! Didn't notice this! But I think #1077 will help in the GitHub Repo Readme and Profile Readme fallback as we can't fetch it from API.

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.

3 participants