Skip to content

E2E Tests: Try localtunnel instead of ngrok #17523

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.

Sign up for GitHub

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

Jump to bottom
Merged
brbrr merged 52 commits into from
Nov 2, 2020
Merged

E2E Tests: Try localtunnel instead of ngrok #17523

brbrr merged 52 commits into from
Nov 2, 2020

Conversation

@brbrr
Copy link
Contributor

@brbrr brbrr commented Oct 19, 2020 •
edited
Loading

Changes proposed in this Pull Request:

  • replace ngrok with self-hosted localtunnel

Jetpack product discussion

n/a

Does this pull request change what data or activity we track or use?

n/a

Testing instructions:

  • Run tests locally:
  • yarn build
  • ./tests/e2e/bin/env.sh start
  • PUPPETEER_HEADLESS=false yarn test-e2e --testPathIgnorePatterns=update
  • Make sure E2Es are passing in CI

Proposed changelog entry for your changes:

  • n/a

@brbrr brbrr self-assigned this Oct 19, 2020
@matticbot
Copy link
Contributor

matticbot commented Oct 19, 2020

Caution: This PR has changes that must be merged to WordPress.com
Hello brbrr! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer and confirm D51398-code works as expected before merging this PR. Once this PR is merged, please commit the changes to WP.com. Thank you!
This revision will be updated with each commit to this PR

@jetpackbot
Copy link
Collaborator

jetpackbot commented Oct 19, 2020 •
edited
Loading

Scheduled Jetpack release: November 10, 2020.
Scheduled code freeze: November 3, 2020

E2E results is available here (for debugging purposes): https://jetpack-e2e-dashboard.herokuapp.com/pr-17523

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Generated by 🚫 dangerJS against a7e3f04

@brbrr brbrr force-pushed the try/e2e-tests-lt branch 2 times, most recently from fc61e13 to 5a72b1c Compare October 26, 2020 12:50
@brbrr brbrr added [Status] Needs Review This PR is ready for review. and removed [Status] In Progress labels Oct 26, 2020
anomiex
anomiex reviewed Oct 26, 2020
View reviewed changes
Copy link
Contributor

@anomiex anomiex left a comment

Choose a reason for hiding this comment

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

I didn't manage to test it (yarn test-e2e as documented in tests/e2e/README.md complained about a MacOS path not existing?), but I gave the code a look-through.

@jeherve jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Oct 27, 2020
@brbrr brbrr mentioned this pull request Oct 28, 2020
Merged
@brbrr brbrr added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Oct 29, 2020
kraftbj
kraftbj approved these changes Oct 29, 2020
View reviewed changes
Copy link
Contributor

@kraftbj kraftbj left a comment

Choose a reason for hiding this comment

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

Didn't independently test and not sure how to confirm localtunnel was used for the GH action. As a test, approving since we will see problems pretty quickly and can fix up.

tests/e2e/bin/wp-setup.sh
touch wp-content/debug.log
chown www-data:www-data wp-content/debug.log

sed -i "/\/\* That's all, stop editing! Happy publishing. \*\//i\
Copy link
Contributor

@kraftbj kraftbj Oct 29, 2020

Choose a reason for hiding this comment

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

Editing this line probably doesn't happen often (last changed 22 months ago), but any harm in looking for the opening <?php line and adding these immediately following it?

Non-blocking.

@kraftbj kraftbj added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Oct 29, 2020
@kraftbj kraftbj added this to the 9.2 milestone Oct 29, 2020
@brbrr brbrr merged commit 45c7aaa into master Nov 2, 2020
@brbrr brbrr deleted the try/e2e-tests-lt branch November 2, 2020 10:55
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Nov 2, 2020
@jeherve
Copy link
Member

jeherve commented Nov 4, 2020

r216323-wpcom

jgcaruso pushed a commit that referenced this pull request Nov 9, 2020
@brbrr @anomiex @jgcaruso
E2E Tests: Try localtunnel instead of ngrok (#17523)
d6f397c 
* add localtunnel dependency

* disable setup script

* start tunnel before running tests

* switch to localtunnel URL

* try closing the tunnel after all

* trigger travis build

* Add TunnelManager

* don't throw for now

* minor updates

* update connection flow

* mess up with cli commands

* cleanup console.log's

* improve in-place connection test stability

* Add a bit of logging

* update wp-config file

* fix script

* 🤷

* 🤷 v2

* 🤷 v3

* bump a timeout

* wait for tunnel to close properly

* wait a bit more

* Retry when getting subdomain

* fix conditional

* update login and fix minor thing

* login tweaks

* more tweaks

* try this@

* random stuff 🤷

* refactor #create method

* a bit more logging

* .

* .

* hack the server

* update teardown

* fix JSON parse error

* minor tweaks

* fix wpcom login

* resolve IDC problem

* pre-review cleanup

* try puppeterr v5

* revert puppeteer update

* pre-review cleanup

* yarn.lock file update

* fix syntax error

* Apply suggestions from code review

Co-authored-by: Brad Jorsch <[email protected]>

* update build setep

* Address feedback, and clean up bin scripts

* Commit new file

* Fix plan sync logging

* capture screenshots for block inserter

* remove screenshots from block-editor

Co-authored-by: Brad Jorsch <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@anomiex anomiex anomiex left review comments

@kraftbj kraftbj kraftbj approved these changes

Assignees

@brbrr brbrr

Labels

E2E Tests Touches WP.com Files

Projects

None yet

Milestone

9.2

Development

Successfully merging this pull request may close these issues.

7 participants

@brbrr @matticbot @jetpackbot @jeherve @kraftbj @anomiex