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

Add playwright script to take screenshots #37

Merged
merged 3 commits into from
Jul 2, 2024

Conversation

signed-long
Copy link
Contributor

@signed-long signed-long commented Jun 23, 2024

Takes screenshots of the login page, demo RouteList, and RouteActivity.

To run:

bun playwright install webkit
bun run dev
node src/ci/screenshots.cjs http://localhost:3000

Also made a follow up to add them to the PR comments: #38

RouteList-mobile
RouteList-desktop
RouteActivity-mobile
RouteActivity-desktop
Login-mobile
Login-desktop

For #36

@adeebshihadeh
Copy link
Contributor

adeebshihadeh commented Jun 23, 2024

Love how concise this is, though it does now introduce managing a separate Python environment now. Can we keep it this simple in JS?

Copy link

Welcome to new-connect! Make sure to:

  • read the contributing guidelines
  • mark your PR as a draft until it's ready to review
  • post the preview on Discord; feedback from users will speedup the PR review

deployed preview: https://37.connect-d5y.pages.dev

@sarem-h
Copy link
Contributor

sarem-h commented Jun 23, 2024

isn't just links to the screenshot better? The PR looks cluttered

@signed-long
Copy link
Contributor Author

isn't just links to the screenshot better? The PR looks cluttered

This change doesn't post them to the PR comments, I just added them here to show the screenshots that the script produces. But yeah we could post links, or just make them smaller in the comment. Working on that in #38

@adeebshihadeh
Copy link
Contributor

Got this even after bun install. Is it expected?

batman:new-connect$ node src/ci/screenshots.cjs http://localhost:3000
node:internal/process/promises:289
            triggerUncaughtException(err, true /* fromPromise */);
            ^

browserType.launch: Executable doesn't exist at /home/batman/.cache/ms-playwright/webkit-2003/pw_run.sh
╔═════════════════════════════════════════════════════════════════════════╗
║ Looks like Playwright Test or Playwright was just installed or updated. ║
║ Please run the following command to download new browsers:              ║
║                                                                         ║
║     npx playwright install                                              ║
║                                                                         ║
║ <3 Playwright Team                                                      ║
╚═════════════════════════════════════════════════════════════════════════╝
    at /home/batman/src/new-connect/src/ci/screenshots.cjs:24:39
    at Object.<anonymous> (/home/batman/src/new-connect/src/ci/screenshots.cjs:34:3) {
  name: 'Error'
}

Node.js v21.1.0

@signed-long
Copy link
Contributor Author

Got this even after bun install. Is it expected?

batman:new-connect$ node src/ci/screenshots.cjs http://localhost:3000
node:internal/process/promises:289
            triggerUncaughtException(err, true /* fromPromise */);
            ^

browserType.launch: Executable doesn't exist at /home/batman/.cache/ms-playwright/webkit-2003/pw_run.sh
╔═════════════════════════════════════════════════════════════════════════╗
║ Looks like Playwright Test or Playwright was just installed or updated. ║
║ Please run the following command to download new browsers:              ║
║                                                                         ║
║     npx playwright install                                              ║
║                                                                         ║
║ <3 Playwright Team                                                      ║
╚═════════════════════════════════════════════════════════════════════════╝
    at /home/batman/src/new-connect/src/ci/screenshots.cjs:24:39
    at Object.<anonymous> (/home/batman/src/new-connect/src/ci/screenshots.cjs:34:3) {
  name: 'Error'
}

Node.js v21.1.0

Ah yeah need to install the browser bun playwright install webkit

@adeebshihadeh adeebshihadeh merged commit e982dd6 into commaai:master Jul 2, 2024
2 checks passed
sarem-h pushed a commit to sarem-h/new-connect that referenced this pull request Jul 6, 2024
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