Skip to content

QVAC-17810 test[skiplog]: add img2img integration tests for diffusion#2241

Merged
Victor-Rodzko merged 2 commits into
mainfrom
test/qvac-17810-img2img-integration-tests
May 26, 2026
Merged

QVAC-17810 test[skiplog]: add img2img integration tests for diffusion#2241
Victor-Rodzko merged 2 commits into
mainfrom
test/qvac-17810-img2img-integration-tests

Conversation

@Victor-Rodzko

Copy link
Copy Markdown
Contributor

🎯 What problem does this PR solve?

  • img2img was shipped to the SDK in QVAC-17304 feat[api]: add img2img support to SDK diffusion API #1662 but tests-qvac only had unit/mock coverage; real integration coverage against loaded diffusion models was missing.
  • tests-qvac/tests/shared/executors/diffusion-executor.ts had drifted: heavy if (testId === ...) branching, unknown/any params, ad-hoc PNG-size byte checks that produce false positives on compressed images.

📝 How does it solve it?

  • New e2e cases in diffusion-tests.ts exercising the img2img path against real loaded models:
    • diffusion-img2img-vs-txt2img-baseline — proves init_image actually changes output (byte-delta + IHDR-dimension comparison vs txt2img baseline).
    • diffusion-img2img-img-cfg-scaleimg_cfg_scale parameter accepted/rendered.
    • diffusion-img2img-invalid-strength — Zod rejects out-of-range strength.
    • Reuses existing diffusion-basic-img2img.
  • Platform split (matches vision-test pattern): asset filename → Uint8Array resolution lives in desktop/executors/diffusion-executor.ts (Node fs); shared executor stays React Native-clean and only sees bytes.
  • Mobile: all diffusion tests skipped (SD 2.1 1B Q8_0 cold-load OOMs Device Farm devices, ~3GB). SkipExecutor message updated; mobile/executors/diffusion-executor.ts removed as dead code.
  • Refactor of shared/executors/diffusion-executor.ts to be a typed reference implementation:
    • Removed execute() override; replaced with a strongly-typed handlers map.
    • Required<{ [K in testId]: HandlerFn<…> }> annotation makes the map exhaustive at compile time — adding a new test without a handler is a TS error.
    • New DiffusionParams interface (no more unknown/any); buildParams/resolveParams typed end-to-end.
    • Consolidated 4 near-duplicate handlers into a single runBasic(resourceKey, …) via bind.
    • Extracted compareWithBaseline helper for img2img-vs-txt2img and fusion-vs-flux2 comparisons.
    • Extracted readPngDims / assertEqualPngDimensions (parse IHDR) so we no longer false-positive on compressed-byte length differences.
  • New minimal asset assets/images/diffusion-img2img-source-256.png (562 B, 256×256 RGB) — keeps SD 2.1 output dimensions matching requested 256×256 and minimizes resource cost.

🧪 How was it tested?

  • Desktop: npm run install:build:full → full diffusion suite green locally (FLUX 2 Klein).
  • iOS (single dev device): img2img cases green locally (full Device Farm run still skipped at consumer level due to OOM).
  • tsc --noEmit clean. Exhaustiveness check verified by removing a handler entry and confirming TS error: Property '"diffusion-standalone-upscaler-x4"' is missing in type … but required in type 'Required<…>'.
  • CI run: https://github.com/tetherto/qvac/actions/runs/26229867278

@Victor-Rodzko Victor-Rodzko requested review from a team as code owners May 25, 2026 08:17
@Victor-Rodzko Victor-Rodzko added e2e-tested Test suite has run on this PR. Does not indicate tests pass/fail - see results in comments. test-e2e-smoke Triggers smoke e2e test suite [Currently SDK-only] tier1 verified Authorize secrets / label-gate in PR workflows verify labels May 25, 2026
@github-actions

github-actions Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

QVAC E2E — android — ✅ all tests passed (83/91, 2826s)

Config: suite=smoke · filter=(none) · exclude=(none)
View run · Artifacts: reports · Device Farm logs

@github-actions

github-actions Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

QVAC E2E — ios — ✅ all tests passed (82/91, 1200s)

Config: suite=smoke · filter=(none) · exclude=(none)
View run · Artifacts: reports · Device Farm logs · Device Farm logs

@github-actions

github-actions Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

QVAC E2E — windows — ✅ all tests passed (91/91, 429s)

Config: suite=smoke · filter=(none) · exclude=(none)
View run · Artifacts: reports

@github-actions

github-actions Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

QVAC E2E — linux — ✅ all tests passed (91/91, 236s)

Config: suite=smoke · filter=(none) · exclude=(none)
View run · Artifacts: reports

@github-actions

github-actions Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

QVAC E2E — macos — ✅ all tests passed (91/91, 309s)

Config: suite=smoke · filter=(none) · exclude=(none)
View run · Artifacts: reports

@simon-iribarren simon-iribarren left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

CI Status

Green. PR validation, SDK checks, build, desktop GPU lanes, Android, and iOS e2e are passing. Release/publish jobs are skipped as expected for PR context.

API Surface & Tagging

No public API changes detected. This is test coverage only, so test[skiplog] is appropriate and no [api], [bc], or [mod] tag is required.

Recommendation

Approve. The img2img coverage is scoped to desktop because mobile diffusion remains intentionally skipped for Device Farm memory pressure, which is fine for this PR.

@github-actions

github-actions Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

Tier-based Approval Status

**PR Tier:** TIER1

**Current Status:** ✅ APPROVED

**Requirements:**
- 1 Team Member approval ✅ (1/1)
- 1 Team Lead OR Management approval ✅ (1/1)



---
*This comment is automatically updated when reviews change.*

NamelsKing
NamelsKing previously approved these changes May 26, 2026
@Victor-Rodzko Victor-Rodzko dismissed stale reviews from NamelsKing and simon-iribarren via 1a385ee May 26, 2026 10:57
@kinsta

kinsta Bot commented May 26, 2026

Copy link
Copy Markdown

Preview deployments for qvac-docs-staging ⚡️

Status Branch preview Commit preview
🔁 Deploying... N/A N/A

Commit: ebb024e63b7c26b99e2c99ccd0614012b1ce8850

Deployment ID: 71bc0f90-16b6-4317-ad3c-3d102afb26b2

Static site name: qvac-docs-staging-fazwv

- New e2e cases in tests-qvac for the img2img path against real loaded
  diffusion models: img2img-vs-txt2img baseline (proves init_image
  changes output via byte-delta + IHDR-dimension comparison),
  img_cfg_scale acceptance, invalid-strength validation. Reuses the
  existing diffusion-basic-img2img case.
- Platform split (matches vision tests): asset filename to Uint8Array
  resolution lives in desktop diffusion executor; shared executor only
  sees bytes and stays React Native-clean.
- Skip all diffusion tests on mobile (SD 2.1 1B Q8_0 cold-load OOMs
  Device Farm devices, ~3GB); shorten SkipExecutor message; remove
  dead mobile diffusion executor and its consumer wiring (resource
  definition, executor handler, and SD_V2_1_1B_Q8_0 import).
- Refactor shared diffusion executor to be a typed reference impl:
  drop execute() override in favour of an exhaustive handlers map
  (Required<...> mapped type — adding a test without a handler is a
  compile error), introduce DiffusionParams interface (no unknown/any),
  consolidate 4 near-duplicate handlers into runBasic(resourceKey, ...)
  via bind, extract compareWithBaseline + readPngDims/assertEqualPng
  Dimensions helpers (replaces brittle byte-length checks).
- Add minimal 256x256 RGB PNG asset (562 B) to keep SD 2.1 output dims
  matching requested 256x256 and minimize resource cost.

Co-authored-by: Cursor <cursoragent@cursor.com>
@Victor-Rodzko Victor-Rodzko force-pushed the test/qvac-17810-img2img-integration-tests branch from 82a46f7 to 5678c06 Compare May 26, 2026 13:30
@Victor-Rodzko

Copy link
Copy Markdown
Contributor Author

/review

@Victor-Rodzko Victor-Rodzko merged commit a03d9dd into main May 26, 2026
25 checks passed
@Victor-Rodzko Victor-Rodzko deleted the test/qvac-17810-img2img-integration-tests branch May 26, 2026 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

e2e-tested Test suite has run on this PR. Does not indicate tests pass/fail - see results in comments. test-e2e-smoke Triggers smoke e2e test suite [Currently SDK-only] tier1 verified Authorize secrets / label-gate in PR workflows verify

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants