Skip to content
New issue

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

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

Already on GitHub? # to your account

Upgrade airtap and use GitHub Actions #443

Merged
merged 2 commits into from
Sep 24, 2020
Merged

Conversation

vweevers
Copy link
Contributor

  • Upgrade airtap to v4 (see its upgrade guide)
  • Move Sauce Labs tests from Travis to GitHub Actions
  • Use playwright for headless local testing

Complements #418.

@vweevers vweevers requested a review from mcollina September 19, 2020 21:02
@vweevers

This comment has been minimized.

vweevers added a commit to airtap/playwright that referenced this pull request Sep 19, 2020
Because playwright has an install step that fails on node < 10.

Ref nodejs/readable-stream#443
@vweevers vweevers force-pushed the vweevers-upgrade-airtap branch from 78fed0c to 405790c Compare September 19, 2020 21:37
@mcollina
Copy link
Member

They keys are set up

- Upgrade airtap to v4
- Move Sauce Labs tests from Travis to GitHub Actions
- Use playwright for headless local testing
@vweevers vweevers force-pushed the vweevers-upgrade-airtap branch from 405790c to ce87b75 Compare September 23, 2020 19:43
@vweevers
Copy link
Contributor Author

Build failed:

Error: 23 Sep 19:50:19 - Sauce Connect could not establish a connection.

Will see if I can replicate that on another repo that has an almost identical setup, and no sauce connect problems.

@vweevers
Copy link
Contributor Author

vweevers commented Sep 23, 2020

With debug output enabled, the problem seems to be the credentials:

ERROR: connecting to https://saucelabs.com/rest/v1/***/tunnels: HTTP response code indicated failure, reply: {"message": "Not authorized"}.
ERROR: can't reach https://saucelabs.com/rest/v1/***/tunnels, please check your username/access key, firewall and proxy settings.

@mcollina Could you please double-check the credentials? You can also try it locally:

SAUCE_USERNAME=xx SAUCE_ACCESS_KEY=xx DEBUG=airtap* npm run test-browsers

# Run a diagnostic test
SAUCE_USERNAME=xx SAUCE_ACCESS_KEY=xx ./node_modules/sauce-connect-launcher/sc/sc-4.6.2-linux/bin/sc --doctor

@mcollina
Copy link
Member

Thanks! Fixed and now the tests are passing!

@mcollina
Copy link
Member

Running npm test-browser I'm getting:

Hostname 'airtap.local' must resolve to 127.0.0.1. Please add an entry to your hosts file.

@mcollina mcollina merged commit 19381e9 into master Sep 24, 2020
@mcollina mcollina deleted the vweevers-upgrade-airtap branch September 24, 2020 07:59
@vweevers
Copy link
Contributor Author

@mcollina That hostname error is expected. Some browsers don't route localhost traffic through the Sauce Connect tunnel. To work around that, airtap uses a custom hostname that defaults to airtap.local and should resolve to 127.0.0.1. Only needed for sauce, not for playwright and others.

The debug steps I added in the last commit should be removed, especially ./node_modules/sauce-connect-launcher/sc/sc-4.6.2-linux/bin/sc which will break when a new Sauce Connect version is released by Sauce Labs.

@mcollina
Copy link
Member

oh gosh... can you send a fresh PR?

vweevers added a commit that referenced this pull request Sep 24, 2020
Remove debug steps from sauce workflow (#443), remove travis, and
replace badges in readme.
@vweevers vweevers mentioned this pull request Sep 24, 2020
mcollina pushed a commit that referenced this pull request Sep 24, 2020
Remove debug steps from sauce workflow (#443), remove travis, and
replace badges in readme.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants