Skip to content

test(previews): unit-test the dev-mobile helpers - #725

Open
Barnabas A Nsoh (ayinloya) wants to merge 2 commits into
mainfrom
test/dev-mobile-helpers-web-client
Open

test(previews): unit-test the dev-mobile helpers#725
Barnabas A Nsoh (ayinloya) wants to merge 2 commits into
mainfrom
test/dev-mobile-helpers-web-client

Conversation

@ayinloya

@ayinloya Barnabas A Nsoh (ayinloya) commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

User description

Mirrors the same change made to our v3 SDK repo's copy of this script, so the two stay in step.

Why

The bug fixed in #724 — an ERE alternation that anchored only its first branch, so an ordinary app log line like user does not exist aborted a healthy deploy — was pure string matching. No AWS, no processes, no timing. It shipped in #651 and passed review. That's the case for covering this code.

Two more helpers carry the same risk profile: env_file_default (parses export K = "v", strips quotes and whitespace, last-wins) and sst_log_plain (a sed pipeline stripping ANSI escapes and turning CRs into newlines).

The refactor this needed

dev-mobile.sh could not be tested as written. Its top-level code runs port preflight, mktemp -d, and eventually a full build + tunnel + sst dev the moment the file is read — so sourcing it to reach a function executes all of that.

The pure helpers now live in previews/scripts/lib/dev-mobile-lib.sh, which is side-effect free to source; dev-mobile.sh sources it. Extracted:

helper change
env_file_default takes the file as an argument instead of reading $PREVIEWS_DIR
sst_log_plain takes a path instead of reading $SST_SERVER_LOG
is_valid_port was an inline [[ =~ ]] test
extract_tunnel_url was an inline grep
sst_log_is_ready was an inline grep
sst_log_has_error was an inline grep

Behaviour is unchanged.

The tests

14 cases under node --test, matching the .test.mjs convention already used across packages/. No new dependency, no test framework. They shell into bash with the library sourced, passing fixtures as positional args so no test data has to survive shell quoting.

Not covered, deliberately: tunnel orchestration, the sst readiness loop, process-group cleanup. Mocking cloudflared, sst and lsof would cost more than it protects.

Verified

The suite was mutation-tested against three deliberate regressions, all caught: restoring the original unanchored alternation, dropping the $ anchor from the Complete readiness check, and making env_file_default first-wins instead of last-wins.

Checked here: bash -n on both files; npm run test (14/14) and npm run lint in previews; prettier clean; and the script still behaves identically — non-numeric port rejected, previews/.env still read.

CI

Adds a previews job to lint.yml running npm run lint and npm run test.

Worth flagging: the repo's existing node --test suites in packages/ are not run by any workflow — I checked all of them. I wired this one up because a test that never runs wouldn't defend against the regression that prompted it. Whether to wire up the others is a separate question, and probably worth someone's attention.

Parity

After this, the two repos' copies of dev-mobile.sh differ only by the workspace name they build (@smileid/web-components here), and dev-mobile-lib.sh / dev-mobile.test.mjs are byte-identical across both.


PR Type

Tests, Enhancement


Description

  • Extract pure dev-mobile.sh helpers into sourceable lib/dev-mobile-lib.sh

  • Add 14 node --test cases covering regexes, env parsing, ports

  • Wire a previews lint + unit-test job into CI

  • Add test script to previews/package.json


Diagram Walkthrough

flowchart LR
  A["dev-mobile.sh (inline helpers)"] -- "extract" --> B["scripts/lib/dev-mobile-lib.sh"]
  B -- "sourced by" --> A
  B -- "sourced by" --> C["dev-mobile.test.mjs (node --test)"]
  C -- "npm run test" --> D["lint.yml previews job"]
Loading

File Walkthrough

Relevant files
Enhancement
dev-mobile-lib.sh
New sourceable library of pure dev-mobile helpers               

previews/scripts/lib/dev-mobile-lib.sh

  • New side-effect-free library of pure shell helpers.
  • Contains env_file_default (now takes file arg), is_valid_port,
    extract_tunnel_url, sst_log_plain (now takes path), sst_log_is_ready,
    sst_log_has_error.
  • Documents why the env file is parsed instead of sourced and why the
    error regex must stay fully anchored.
+72/-0   
dev-mobile.sh
Delegate string helpers to extracted library                         

previews/scripts/dev-mobile.sh

  • Adds SCRIPT_DIR and sources lib/dev-mobile-lib.sh.
  • Removes inline env_file_default and sst_log_plain definitions; calls
    them with explicit path arguments.
  • Replaces inline regex checks with is_valid_port, extract_tunnel_url,
    sst_log_is_ready, sst_log_has_error.
  • Behaviour unchanged; comments relocated to the library.
+16/-43 
Tests
dev-mobile.test.mjs
Unit tests for dev-mobile shell helpers                                   

previews/scripts/dev-mobile.test.mjs

  • Adds 14 node --test cases exercising the extracted helpers via bash.
  • Pins the anchored-alternation regression: app log lines mentioning
    errors must not abort.
  • Covers ready-line detection, ANSI/CR normalization, env-file parsing
    (last-wins, prefix keys, comments), port validation and tunnel URL
    extraction.
  • Uses temp-file fixtures and passes test data as positional args to
    avoid shell quoting issues.
+221/-0 
Configuration changes
lint.yml
Add previews lint and unit-test CI job                                     

.github/workflows/lint.yml

  • Adds a previews job running in ./previews.
  • Runs npm ci, npm run lint and npm run test.
+16/-0   
package.json
Add test script for previews package                                         

previews/package.json

  • Adds test script running node --test scripts/*.test.mjs.
+1/-0     


Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • Mirrors smileidentity/web-sdk#71, keeping the two copies of this script in
    step.
    
    The bug fixed in #724 — an ERE alternation that anchored only its first
    branch, so ordinary app log lines aborted a healthy deploy — was pure string
    matching. It shipped in #651 and passed review, which is a good argument for
    covering this code.
    
    dev-mobile.sh could not be tested as written: its top-level code runs port
    preflight, `mktemp -d` and eventually a full build, tunnel and `sst dev` the
    moment the file is read, so sourcing it to reach a function is not an
    option. Move the pure helpers into scripts/lib/dev-mobile-lib.sh, which is
    side-effect free to source, and have the script source it.
    
    Extracted: env_file_default (now takes the file as an argument rather than
    reading $PREVIEWS_DIR), is_valid_port, extract_tunnel_url, sst_log_plain
    (likewise now takes a path), sst_log_is_ready, sst_log_has_error. The last
    two were previously inline greps. Behaviour is unchanged.
    
    Adds 14 node --test cases covering the regexes, the env-file parsing and
    port validation, matching the .test.mjs convention already used in
    packages/. Deliberately not covered: tunnels, the sst readiness loop and
    process-group cleanup — mocking cloudflared and sst would cost more than it
    protects.
    
    Wires `npm run test` into a previews job in lint.yml. The repo's existing
    node --test suites are not run by any workflow; this one is, otherwise it
    would not defend against the regression that prompted it.
    Comment thread .github/workflows/lint.yml Fixed
    @github-actions

    Copy link
    Copy Markdown

    🔍 Semgrep Security Scan Results

    ✅ No security findings detected by p/security-audit ruleset.

    @prfectionist

    prfectionist Bot commented Aug 14, 2026

    Copy link
    Copy Markdown
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis ✅

    724 - Fully compliant

    Compliant requirements:

    • Anchored group preserved verbatim in sst_log_has_error.
    • Genuine failures still matched (covered by new tests).
    • Comment warning against "simplifying" the regex retained and expanded.

    651 - PR Code Verified

    Compliant requirements:

    • Env-file parsing and numeric validation preserved (now via env_file_default / is_valid_port).

    Requires further human verification:

    • Whether the refactored script still behaves identically end-to-end (tunnels, preflight, cleanup) needs a manual npm run dev:mobile run; the new tests cover only the pure helpers.
    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🏅 Score: 85
    🧪 PR contains tests
    ⚡ Recommended focus areas for review

    CI Prerequisite

    The new previews job runs npm ci with working-directory: ./previews, which requires a committed previews/package-lock.json (and, unlike other jobs here, it also installs a separate dependency tree just to run bash-only unit tests). If previews has no lock file of its own — it is not covered by the root workspace globs — this job will fail on every PR. Worth confirming the lock file exists, or install from the repo root instead.

    previews:
      runs-on: ubuntu-latest
      defaults:
        run:
          working-directory: ./previews
      steps:
        - name: checkout code
          uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
        - name: set node version
          uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v6
        - name: install dependencies
          run: npm ci
        - name: lint
          run: npm run lint
        - name: unit tests
          run: npm run test

    @prfectionist

    prfectionist Bot commented Aug 14, 2026

    Copy link
    Copy Markdown
    Contributor

    PR Code Suggestions ✨

    No code suggestions found for the PR.

    @github-actions

    Copy link
    Copy Markdown

    This branch has been deployed to s3 / cloudfront.

    ✅ Preview URL for Smart Camera Web:

    https://cdn.smileidentity.com/js/preview-test/dev-mobile-helpers-web-client/smart-camera-web.js
    

    ✅ Preview URL for Embed:

    https://cdn.smileidentity.com/inline/preview-test/dev-mobile-helpers-web-client/js/script.min.js
    

    ✅ Preview URL for Web Client (Sandbox):

    https://d3b2kmlu9oway3.cloudfront.net
    

    ✅ Preview URL for Web Client (Production):

    https://d3n94res6uybfr.cloudfront.net
    

    …ntain permissions'
    
    Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
    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.

    2 participants