Skip to content

Add Jest + Playwright BrowserStack SDK sample#2

Open
saraswati-padmawar wants to merge 8 commits into
mainfrom
add-jest-playwright-sample
Open

Add Jest + Playwright BrowserStack SDK sample#2
saraswati-padmawar wants to merge 8 commits into
mainfrom
add-jest-playwright-sample

Conversation

@saraswati-padmawar

@saraswati-padmawar saraswati-padmawar commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Populates this customer-facing sample repo (previously skeleton: README + CODEOWNERS + Semgrep only) with a working Jest + Playwright sample driven by the BrowserStack Node SDK.

What's included

  • tests/bstack_sample.test.js — public bstackdemo.com add-to-cart scenario
  • tests/bstack_local_test.test.js — BrowserStack Local scenario (bs-local.com:45454)
  • browserstack.yml — credentials, 3 platforms (Win/Chrome, macOS/WebKit, Win/Firefox), Local toggle, framework: playwright, testReporting: true
  • package.jsonsample-test / sample-local-test scripts via browserstack-node-sdk jest
  • jest.config.js / jest.local.config.js, .gitignore, README

References

  • Follows the standardized Automate sample template (Confluence: SDK Sample Repo Generator, R1–R4)
  • SDK run command per the node-agent framework-matrix.md (browserstack-node-sdk jest)
  • framework: playwright mirrors the existing codecept-js-playwright-browserstack precedent
  • Structure mirrors node-js-playwright-browserstack

Verified on BrowserStack

Project: BrowserStack Samples.

The public scenario is fully verified. The Local scenario could not be confirmed in the local environment used for verification — the SDK's BrowserStack Local tunnel was not connected by the time the parallel Jest workers called chromium.launch() ([browserstack.local] is set to true but local testing through BrowserStack is not connected). The same Local scenario passes in the sibling cucumber-js and nunit samples with identical config, so this is a tunnel-startup timing issue in that environment, not a defect in the sample's test code or browserstack.yml. The included sanity-workflow.yml runs the Local step on a clean GitHub runner — please confirm it green there before merge.

Note from verification: the sample must depend on @playwright/test (not playwright) for the SDK to detect & route Playwright — reflected in this PR.

🤖 Generated with Claude Code

Sample test (bstackdemo add-to-cart) + local test (BrowserStack Local via
bs-local.com:45454), browserstack.yml, package.json with browserstack-node-sdk
run scripts, and README. Follows the standardized Automate sample template.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@saraswati-padmawar saraswati-padmawar requested a review from a team as a code owner June 30, 2026 08:16
saraswati-padmawar and others added 3 commits June 30, 2026 14:04
The Node SDK's jest flow gates Playwright support on resolving @playwright/test
from node_modules (src/bin/jest/command.js: getPackageVersion('@playwright/test')).
With only 'playwright' installed, detection never fired, the SDK skipped its
BrowserType.launch patch, and chromium.launch() ran locally instead of routing to
BrowserStack. Switch the dependency and imports to @playwright/test.

Verified: chromium.launch() now routes to wss://cdp.browserstack.com/playwright.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Mirrors browserstack/node-js-playwright-browserstack. Runs the public bstackdemo
sample test + BrowserStack Local test against a python http.server harness, using
org BROWSERSTACK_USERNAME/ACCESS_KEY secrets. Strips yml credential placeholders so
the secrets take effect.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Rohannagariya1
Rohannagariya1 previously approved these changes Jul 1, 2026

@Rohannagariya1 Rohannagariya1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM ✅ — public scenario verified and the sample is clean. Approving.

One thing worth confirming before merge (non-blocking on the code itself):

  • The Local scenario is unverified due to the SDK tunnel not being connected when the parallel Jest workers call chromium.launch(). The "passes in the cucumber/nunit siblings" argument doesn't fully carry over since those aren't multi-worker Jest — the tunnel-startup vs parallel-worker race can reproduce on the runner too. Please confirm the sanity workflow is green on the Local step before merging; if it flakes, maxWorkers: 1 in jest.local.config.js would serialize the workers and sidestep it.

Optional/minor:

  • latest deps + postinstall: npm update + gitignored lockfile = non-reproducible installs (matches BrowserStack sample convention, fine to keep).

osho-20
osho-20 previously approved these changes Jul 7, 2026
The Local scenario navigates to bs-local.com:45454 and asserts the page title
contains 'BrowserStack Local', but the README never told users to serve anything
on that port -- so following it verbatim failed. Document serving a matching local
page (and how to point at your own app), and describe the tunnel routing accurately.
@saraswati-padmawar saraswati-padmawar dismissed stale reviews from osho-20 and Rohannagariya1 via 8e7c834 July 9, 2026 09:39
Comment thread .github/workflows/sanity-workflow.yml Fixed
Comment thread .github/workflows/sanity-workflow.yml Fixed
@saraswati-padmawar

Copy link
Copy Markdown
Collaborator Author

Claude Code PR Review

PR: #2Head: 8e7c834Reviewers: stack:code-reviewer

Summary

Populates this previously-skeleton sample repo with a working Jest + Playwright sample driven by the BrowserStack Node SDK. The public bstackdemo scenario is verified green across 3 platforms; the BrowserStack Local scenario is pending CI confirmation.

Review Table

Priority Category Check Status Notes
High Security No hardcoded secrets or credentials Pass Placeholders only; org secrets in workflow
High Security Authentication/authorization checks present N/A Sample test project
High Security Input validation and sanitization N/A
High Security No IDOR — resource ownership validated N/A
High Security No SQL injection (parameterized queries) N/A No DB
High Correctness Logic is correct, handles edge cases Pass testMatch patterns correct; public run verified green
High Correctness Error handling is explicit, no swallowed exceptions Pass beforeAll/afterAll manage browser lifecycle
High Correctness No race conditions or concurrency issues Pass Per-suite browser; SDK fans platforms into separate workers
Medium Testing New code has corresponding tests Pass The sample is the test
Medium Testing Error paths and edge cases tested N/A Sample scope
Medium Testing Existing tests still pass (no regressions) Pass Fresh repo
Medium Performance No N+1 queries or unbounded data fetching N/A
Medium Performance Long-running tasks use background jobs N/A
Medium Quality Follows existing codebase patterns Pass Mirrors node-js-playwright-browserstack
Medium Quality Changes are focused (single concern) Pass
Low Quality Meaningful names, no dead code Pass except main: index.js (below)
Low Quality Comments explain why, not what Pass
Low Quality No unnecessary dependencies added Pass @playwright/test required for SDK PW detection

Findings

README.md Medium — Local test not runnable as documented (no server on :45454; title assertion coupled to CI harness) Resolved — fixed in 8e7c834: README now documents serving a matching local page on 45454 and adapting to your own app.

  • File: tests/bstack_local_test.test.js:31 + browserstack.yml

  • Severity: Medium

  • Reviewer: stack:code-reviewer

  • Issue: the BrowserStack Local scenario is not yet verified end-to-end (SDK Local tunnel-startup timing in the verification environment); the public scenario is verified green.

  • Suggestion: treat a green sanity-workflow.yml Local step on the runner as a merge gate before relying on the Local path.

  • File: package.json:5

  • Severity: Low

  • Reviewer: stack:code-reviewer

  • Issue: "main": "index.js" references a file that does not exist (leftover from npm init).

  • Suggestion: remove the main field (and the unused keywords/author boilerplate).

  • File: jest.config.js:7 / jest.local.config.js:6

  • Severity: Low

  • Reviewer: stack:code-reviewer

  • Issue: testTimeout: 90s includes remote session startup + nav + assertions; may be tight under session queueing on a busy account.

  • Suggestion: consider 120s for headroom.

  • File: .github/workflows/sanity-workflow.yml:19

  • Severity: Low

  • Reviewer: stack:code-reviewer

  • Issue: CI matrix only exercises Node 20 though README advertises 18/20/22.

  • Suggestion: add 18/22 to the matrix, or keep the "verified on Node 20" caveat prominent.


Verdict: PASS — no High/Critical issues; the public scenario is verified green, testMatch and SDK-detection logic are correct, and the workflow has no injection risk. Confirm the Local step on CI before merge.

Review run via the BrowserStack AI harness stack:pr-review methodology (harness not installed in the run environment → independent stack:code-reviewer fallback).

Semgrep's supply-chain rule (action not pinned to a full commit SHA) blocks on
unpinned tag refs like actions/checkout@v4. Pin all actions to SHAs with a
version comment, matching the repo's Semgrep.yml. Also modernizes v3->v4.
Reviewer specified the onboarding-funnel source tag as <repo>:main:v1.0 -- the
middle segment is the branch the sample lives on (main), not the 'sample-sdk' label.
…ecklist

The canonical format (Confluence ENG/4080468210) is {f/w-name}:sample-{branch-name}:v{version}.
The sample lives on main, so the branch segment is sample-main (keeps the sample- prefix).
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.

4 participants