diff --git a/README.md b/README.md index 220c48a0..58d2cb73 100644 --- a/README.md +++ b/README.md @@ -144,18 +144,19 @@ test('homepage looks correct', async ({ page }) => { fullPage: true, requestTimeout: 5000, properties: { - browser: 'chrome', - viewport: { width: 1920, height: 1080 }, + theme: 'dark', + locale: 'en-US', }, }); }); ``` -`properties` is your metadata bag for baseline grouping, filtering, and -debugging. SDK options such as `threshold`, `minClusterSize`, `fullPage`, -`requestTimeout`, and `buildId` stay at the top level. Reserved SDK and capture -fields inside `properties` are ignored and produce a warning; they are never -interpreted as options or stored as user metadata. +`properties` is your key/value metadata bag for baseline grouping, filtering, +and debugging. Vizzly passes every property through as user metadata; property +names are never interpreted as options. The supported top-level options are +`threshold`, `minClusterSize`, `fullPage`, `requestTimeout`, and `buildId`. +Vizzly reads width and height from the captured image, so viewport dimensions do +not need to be included in `properties`. The client SDK is lightweight. It posts screenshots to the local Vizzly server or the cloud build wrapper. It works with any test runner. diff --git a/clients/ember/README.md b/clients/ember/README.md index 69106f31..93ec6a9c 100644 --- a/clients/ember/README.md +++ b/clients/ember/README.md @@ -222,9 +222,8 @@ await vizzlyScreenshot('screenshot-name', { setting injected from `VIZZLY_FAIL_ON_DIFF` or `.vizzly/server.json`, then non-failing mode. -Each screenshot also includes Vizzly metadata for grouping and comparison: -`browser`, `viewport_width`, `viewport_height`, `url`, and any custom -`properties` you provide. +Each screenshot also includes `browser`, `url`, and any custom `properties` you +provide. Vizzly reads dimensions from the captured image. The function automatically: - Waits for Ember's `settled()` before capturing diff --git a/clients/ember/src/test-support/index.js b/clients/ember/src/test-support/index.js index e63e6d98..c4462fe6 100644 --- a/clients/ember/src/test-support/index.js +++ b/clients/ember/src/test-support/index.js @@ -269,30 +269,13 @@ export async function vizzlyScreenshot(name, options = {}) { // eslint-disable-next-line no-unused-expressions document.body.offsetHeight; - let customViewport = properties.viewport; - let customViewportWidth = properties.viewport_width; - let customViewportHeight = properties.viewport_height; let screenshotProperties = { - ...properties, framework: 'ember', browser: detectBrowser(), - viewport_width: width, - viewport_height: height, url: window.location.href, + ...properties, }; - if (customViewport !== undefined) { - screenshotProperties.viewport = customViewport; - } - - if (customViewportWidth !== undefined) { - screenshotProperties.viewport_width = customViewportWidth; - } - - if (customViewportHeight !== undefined) { - screenshotProperties.viewport_height = customViewportHeight; - } - // Build request payload let payload = { buildId: buildId || window.__VIZZLY_BUILD_ID__ || null, diff --git a/clients/ember/tests/unit/test-support.test.js b/clients/ember/tests/unit/test-support.test.js index ff9a82d8..4126c079 100644 --- a/clients/ember/tests/unit/test-support.test.js +++ b/clients/ember/tests/unit/test-support.test.js @@ -101,8 +101,6 @@ describe('test-support', () => { assert.deepStrictEqual(capturedBody.properties, { framework: 'ember', browser: 'chromium', - viewport_width: 1440, - viewport_height: 900, url: 'http://localhost:4200/dashboard', theme: 'dark', }); @@ -110,7 +108,7 @@ describe('test-support', () => { assert.strictEqual(capturedBody.minClusterSize, 10); }); - it('keeps reserved metadata stable while allowing custom viewport metadata', async () => { + it('preserves viewport keys when the user explicitly supplies them', async () => { let capturedBody = null; installBrowserGlobals(async (_url, request) => { @@ -136,9 +134,9 @@ describe('test-support', () => { }); assert.deepStrictEqual(capturedBody.properties, { - framework: 'ember', - browser: 'chromium', - url: 'http://localhost:4200/dashboard', + framework: 'custom-framework', + browser: 'webkit', + url: 'http://evil.example', theme: 'dark', viewport: { width: 375, height: 667 }, viewport_width: 375, diff --git a/clients/ruby/README.md b/clients/ruby/README.md index 4b0de3a5..552251ca 100644 --- a/clients/ruby/README.md +++ b/clients/ruby/README.md @@ -41,7 +41,8 @@ Vizzly.screenshot('homepage', image_data) Vizzly.screenshot('checkout-page', image_data, properties: { browser: 'chrome', - viewport: { width: 1920, height: 1080 } + theme: 'dark', + locale: 'en-US' }, threshold: 5, min_cluster_size: 3, @@ -104,7 +105,8 @@ RSpec.describe 'Homepage', type: :feature do Vizzly.screenshot('checkout-form', image_data, properties: { browser: 'chrome', - viewport: { width: 1920, height: 1080 } + theme: 'dark', + locale: 'en-US' } ) end diff --git a/clients/ruby/example/test_screenshot.rb b/clients/ruby/example/test_screenshot.rb index 4eaf204f..6f813f31 100755 --- a/clients/ruby/example/test_screenshot.rb +++ b/clients/ruby/example/test_screenshot.rb @@ -32,7 +32,8 @@ def test_captures_vizzly_homepage result = Vizzly.screenshot('vizzly-homepage', image_data, properties: { browser: 'chrome', - viewport: { width: 1920, height: 1080 } + theme: 'light', + locale: 'en-US' }) puts "\n✓ Screenshot captured!" diff --git a/clients/ruby/lib/vizzly.rb b/clients/ruby/lib/vizzly.rb index a436221f..dfea814b 100644 --- a/clients/ruby/lib/vizzly.rb +++ b/clients/ruby/lib/vizzly.rb @@ -10,28 +10,6 @@ class Error < StandardError; end # Default port for local TDD server DEFAULT_TDD_PORT = 47392 - RESERVED_PROPERTY_OPTIONS = %w[ - threshold - min_cluster_size - minClusterSize - full_page - fullPage - capture_mode - captureMode - device_scale_factor - deviceScaleFactor - pixelRatio - dpr - selector - component - element_selector - elementSelector - build_id - buildId - request_timeout - requestTimeout - ].freeze - class Client # rubocop:disable Metrics/ClassLength attr_reader :server_url, :disabled @@ -66,7 +44,7 @@ def initialize(server_url: nil, fail_on_diff: nil) # # @example With options # client.screenshot('checkout', image_data, - # properties: { browser: 'chrome', viewport: { width: 1920, height: 1080 } }, + # properties: { theme: 'dark', locale: 'en-US' }, # threshold: 5 # ) def screenshot(name, image_data, options = {}) # rubocop:disable Metrics @@ -82,8 +60,6 @@ def screenshot(name, image_data, options = {}) # rubocop:disable Metrics options = normalize_options(options) normalized = normalize_screenshot_options(options) - normalized[:warnings].each { |warning| warn warning[:message] } - request_timeout = normalized[:request_timeout] request_timeout_seconds = request_timeout ? request_timeout.to_f / 1000.0 : 30 build_id = normalized[:build_id] || ENV.fetch('VIZZLY_BUILD_ID', nil) @@ -96,8 +72,7 @@ def screenshot(name, image_data, options = {}) # rubocop:disable Metrics properties: normalized[:properties], threshold: normalized[:threshold], minClusterSize: normalized[:minClusterSize], - fullPage: normalized[:fullPage], - warnings: normalized[:warnings] + fullPage: normalized[:fullPage] }.compact uri = URI("#{@server_url}/screenshot") @@ -270,16 +245,6 @@ def normalize_screenshot_options(options) build_id = option_value(options, :build_id, :buildId) request_timeout = option_value(options, :request_timeout, :requestTimeout) properties = options[:properties] || {} - warnings = [] - - properties = properties.each_with_object({}) do |(key, value), normalized_properties| - option = key.to_s - if RESERVED_PROPERTY_OPTIONS.include?(option) - warnings << reserved_property_warning(option) - else - normalized_properties[key] = value - end - end { build_id: build_id, @@ -287,16 +252,7 @@ def normalize_screenshot_options(options) properties: properties, threshold: threshold, minClusterSize: min_cluster_size, - fullPage: full_page, - warnings: warnings - } - end - - def reserved_property_warning(option) - { - code: 'reserved-property-option', - option: option, - message: "Move \"#{option}\" out of properties; properties is only for user metadata." + fullPage: full_page } end diff --git a/clients/ruby/test/vizzly_test.rb b/clients/ruby/test/vizzly_test.rb index 8f4fcda2..6788738e 100644 --- a/clients/ruby/test/vizzly_test.rb +++ b/clients/ruby/test/vizzly_test.rb @@ -236,6 +236,7 @@ def test_screenshot_serializes_fractional_threshold_separately_from_properties captured_body = JSON.parse(request.body) response end + Net::HTTP.define_singleton_method(:start) do |_host, _port, **_options, &block| block.call(fake_http) end @@ -330,7 +331,7 @@ def test_screenshot_accepts_string_keys_and_preserves_zero_values Net::HTTP.define_singleton_method(:start, original_start) end - def test_screenshot_does_not_promote_reserved_options_from_properties + def test_screenshot_preserves_option_shaped_user_properties_without_promoting_them captured_body = nil original_start = Net::HTTP.method(:start) response = Net::HTTPOK.new('1.1', '200', 'OK') @@ -345,7 +346,7 @@ def test_screenshot_does_not_promote_reserved_options_from_properties end client = Vizzly::Client.new(server_url: 'http://localhost:47392') - result = capture_io do + capture_io do client.screenshot( 'reserved-properties', 'fake_image_data', @@ -358,16 +359,12 @@ def test_screenshot_does_not_promote_reserved_options_from_properties ) end - assert_match(/Move "threshold" out of properties/, result[1]) refute_equal 'build-from-properties', captured_body['buildId'] assert_equal 'dark', captured_body['properties']['theme'] - refute_includes captured_body['properties'], 'threshold' - refute_includes captured_body['properties'], 'minClusterSize' - refute_includes captured_body['properties'], 'buildId' - assert_equal( - %w[threshold minClusterSize buildId], - captured_body['warnings'].map { |warning| warning['option'] } - ) + assert_equal 1.5, captured_body['properties']['threshold'] + assert_equal 3, captured_body['properties']['minClusterSize'] + assert_equal 'build-from-properties', captured_body['properties']['buildId'] + refute_includes captured_body, 'warnings' ensure Net::HTTP.define_singleton_method(:start, original_start) end diff --git a/clients/static-site/README.md b/clients/static-site/README.md index a58af079..e54ef0ec 100644 --- a/clients/static-site/README.md +++ b/clients/static-site/README.md @@ -252,23 +252,21 @@ Patterns support glob-like syntax: ## Screenshot Naming -Screenshots are named based on the page path. The plugin records browser, -viewport, viewport dimensions, page URL, and capture mode metadata -automatically. You can add custom screenshot `properties` from config when you -need extra signature dimensions such as theme, locale, or auth state: +Screenshots are named based on the page path. The plugin records the browser and +page URL automatically. Vizzly reads the image dimensions from the captured +image. You can add custom screenshot `properties` from config when you need +extra signature values such as theme, locale, or auth state: **Name format:** `path-to-page` (slashes replaced with hyphens) -**Properties:** Browser, viewport, URL, capture-mode metadata, and any custom -properties (`browser`, `viewport`, `viewport_width`, `viewport_height`, `url`, -`fullPage`, plus user-defined fields) +**Properties:** Browser, URL, and any custom user-defined fields Examples: -- Name: `index`, Properties: `{ browser: 'chromium', viewport: 'mobile', viewport_width: 375, viewport_height: 667, url: 'http://localhost:3000/' }` -- Name: `blog-post-1`, Properties: `{ browser: 'chromium', viewport: 'desktop', viewport_width: 1920, viewport_height: 1080, url: 'http://localhost:3000/blog/post-1' }` -- Name: `docs-getting-started`, Properties: `{ browser: 'webkit', viewport: 'tablet', viewport_width: 768, viewport_height: 1024, url: 'http://localhost:3000/docs/getting-started' }` +- Name: `index`, Properties: `{ browser: 'chromium', url: 'http://localhost:3000/' }` +- Name: `blog-post-1`, Properties: `{ browser: 'chromium', url: 'http://localhost:3000/blog/post-1' }` +- Name: `docs-getting-started`, Properties: `{ browser: 'webkit', url: 'http://localhost:3000/docs/getting-started' }` -This approach allows Vizzly to group screenshots by viewport while keeping names clean and compatible with file system restrictions. +Static-site screenshot names stay clean and compatible with file system restrictions. ## Visual Development Workflow diff --git a/clients/static-site/src/screenshot.js b/clients/static-site/src/screenshot.js index 7e565298..2ff67d60 100644 --- a/clients/static-site/src/screenshot.js +++ b/clients/static-site/src/screenshot.js @@ -42,17 +42,13 @@ export function generateScreenshotName(page) { } /** - * Generate screenshot properties from viewport - * Properties are used by Vizzly for grouping and identification - * @param {Object} viewport - Viewport object with name, width, height + * Generate screenshot properties for the captured page. + * Image dimensions are read from the captured image by Vizzly. + * @param {Object} _viewport - Capture viewport, intentionally not serialized * @returns {Object} Screenshot properties */ -export function generateScreenshotProperties(viewport, options = {}) { - let properties = { - viewport: viewport.name, - viewport_width: viewport.width, - viewport_height: viewport.height, - }; +export function generateScreenshotProperties(_viewport, options = {}) { + let properties = {}; if (options.browser) { properties.browser = options.browser; diff --git a/clients/static-site/tests/screenshot.test.js b/clients/static-site/tests/screenshot.test.js index fbd20159..cf86300e 100644 --- a/clients/static-site/tests/screenshot.test.js +++ b/clients/static-site/tests/screenshot.test.js @@ -97,7 +97,7 @@ describe('generateScreenshotName', () => { }); describe('generateScreenshotProperties', () => { - it('generates properties with viewport info', () => { + it('generates browser, URL, and user properties without capture dimensions', () => { let viewport = { name: 'mobile', width: 375, height: 667 }; let properties = generateScreenshotProperties(viewport, { browser: 'firefox', @@ -107,9 +107,6 @@ describe('generateScreenshotProperties', () => { }); assert.deepStrictEqual(properties, { - viewport: 'mobile', - viewport_width: 375, - viewport_height: 667, browser: 'firefox', url: 'http://localhost:3000/mobile', page: 'homepage', @@ -134,7 +131,7 @@ describe('generateScreenshotProperties', () => { assert.strictEqual(properties.fullPage, undefined); }); - it('generates viewport dimensions that cloud SHA checks consume', () => { + it('does not add dimensions to cloud SHA checks', () => { let properties = generateScreenshotProperties({ name: 'mobile', width: 375, @@ -143,41 +140,21 @@ describe('generateScreenshotProperties', () => { let check = buildScreenshotCheckObject('sha-123', 'index', properties); - assert.strictEqual(check.viewport_width, 375); - assert.strictEqual(check.viewport_height, 667); - }); - - it('includes viewport dimensions', () => { - let viewport1 = { name: 'mobile', width: 375, height: 667 }; - let viewport2 = { name: 'desktop', width: 1920, height: 1080 }; - let viewport3 = { name: 'tablet', width: 768, height: 1024 }; - - let props1 = generateScreenshotProperties(viewport1); - let props2 = generateScreenshotProperties(viewport2); - let props3 = generateScreenshotProperties(viewport3); - - assert.strictEqual(props1.viewport_width, 375); - assert.strictEqual(props1.viewport_height, 667); - - assert.strictEqual(props2.viewport_width, 1920); - assert.strictEqual(props2.viewport_height, 1080); - - assert.strictEqual(props3.viewport_width, 768); - assert.strictEqual(props3.viewport_height, 1024); + assert.deepStrictEqual(check, { + sha256: 'sha-123', + name: 'index', + properties: {}, + }); }); - it('handles different viewport names', () => { - let viewport1 = { name: 'mobile', width: 375, height: 667 }; - let viewport2 = { name: 'desktop', width: 1920, height: 1080 }; + it('does not serialize viewport names or dimensions', () => { + let properties = generateScreenshotProperties({ + name: 'mobile', + width: 375, + height: 667, + }); - assert.strictEqual( - generateScreenshotProperties(viewport1).viewport, - 'mobile' - ); - assert.strictEqual( - generateScreenshotProperties(viewport2).viewport, - 'desktop' - ); + assert.deepStrictEqual(properties, {}); }); }); @@ -210,9 +187,6 @@ describe('captureAndSendScreenshot', () => { assert.strictEqual(image, screenshot); assert.deepStrictEqual(options, { properties: { - viewport: 'desktop', - viewport_width: 1920, - viewport_height: 1080, browser: 'chromium', url: 'http://localhost:3000/docs', page: 'docs', diff --git a/clients/storybook/src/screenshot.js b/clients/storybook/src/screenshot.js index e9fe7233..66c30f7f 100644 --- a/clients/storybook/src/screenshot.js +++ b/clients/storybook/src/screenshot.js @@ -35,7 +35,7 @@ export function generateScreenshotName(story, viewport) { export function generateScreenshotProperties( story, - viewport, + _viewport, url, screenshotOptions = {} ) { @@ -43,9 +43,6 @@ export function generateScreenshotProperties( storyId: story.id, storyTitle: story.title, storyName: story.name, - viewport: viewport.name, - viewport_width: viewport.width, - viewport_height: viewport.height, ...(screenshotOptions.browser !== undefined ? { browser: screenshotOptions.browser } : {}), diff --git a/clients/storybook/tests/screenshot.test.js b/clients/storybook/tests/screenshot.test.js index 66647c71..3effa652 100644 --- a/clients/storybook/tests/screenshot.test.js +++ b/clients/storybook/tests/screenshot.test.js @@ -41,7 +41,7 @@ describe('generateScreenshotName', () => { }); describe('generateScreenshotProperties', () => { - it('builds cloud-compatible story and viewport metadata', () => { + it('builds story metadata without serializing capture dimensions', () => { let story = { id: 'button--primary', title: 'Button', name: 'Primary' }; let viewport = { name: 'mobile', width: 375, height: 667 }; let properties = generateScreenshotProperties( @@ -60,9 +60,6 @@ describe('generateScreenshotProperties', () => { storyId: 'button--primary', storyTitle: 'Button', storyName: 'Primary', - viewport: 'mobile', - viewport_width: 375, - viewport_height: 667, url: 'http://localhost:6006/iframe.html?id=button--primary', browser: 'webkit', }); @@ -146,7 +143,7 @@ describe('captureScreenshot', () => { }); describe('captureAndSendScreenshot', () => { - it('should send story and viewport metadata for isolated story preview', async () => { + it('should send story metadata for the isolated story preview', async () => { let mockVizzly = mock.fn(async () => {}); let mockBuffer = Buffer.from('fake-screenshot'); @@ -174,9 +171,6 @@ describe('captureAndSendScreenshot', () => { storyId: 'button--primary', storyTitle: 'Button', storyName: 'Primary', - viewport: 'desktop', - viewport_width: 1920, - viewport_height: 1080, url: iframeUrl, }); }); diff --git a/clients/swift/Sources/Vizzly/VizzlyClient.swift b/clients/swift/Sources/Vizzly/VizzlyClient.swift index 0a6f591c..5c830fcb 100644 --- a/clients/swift/Sources/Vizzly/VizzlyClient.swift +++ b/clients/swift/Sources/Vizzly/VizzlyClient.swift @@ -113,7 +113,7 @@ public final class VizzlyClient { /// - Parameters: /// - name: Unique name for the screenshot /// - image: PNG image data - /// - properties: Additional properties to attach (browser, viewport, etc.) + /// - properties: User properties to attach, such as theme, locale, or state /// - threshold: Optional CIEDE2000 Delta E threshold. When nil, the /// Vizzly server configuration is used. /// - minClusterSize: Optional minimum changed-pixel cluster size to count diff --git a/clients/swift/Tests/VizzlyTests/VizzlyClientTests.swift b/clients/swift/Tests/VizzlyTests/VizzlyClientTests.swift index 2589191e..7b573891 100644 --- a/clients/swift/Tests/VizzlyTests/VizzlyClientTests.swift +++ b/clients/swift/Tests/VizzlyTests/VizzlyClientTests.swift @@ -163,6 +163,7 @@ final class VizzlyClientTests: XCTestCase { image: createTestImage(), properties: [ "browser": "chrome", + "properties": ["threshold": 9], "viewport": [ "width": 1920, "height": 1080 @@ -172,6 +173,7 @@ final class VizzlyClientTests: XCTestCase { let properties = payload["properties"] as? [String: Any] XCTAssertEqual(properties?["browser"] as? String, "chrome") + XCTAssertEqual((properties?["properties"] as? [String: Int])?["threshold"], 9) let viewport = properties?["viewport"] as? [String: Any] XCTAssertEqual(viewport?["width"] as? Int, 1920) diff --git a/clients/vitest/README.md b/clients/vitest/README.md index 6531a901..e57e3e4d 100644 --- a/clients/vitest/README.md +++ b/clients/vitest/README.md @@ -147,12 +147,10 @@ await expect(page).toMatchScreenshot('screenshot.png', { - Playwright/Vitest screenshot options such as `animations`, `caret`, `mask`, `maskColor`, `omitBackground`, `scale`, and `timeout` are passed through to the browser screenshot capture. -- Vizzly automatically adds `browser`, `url`, `viewport`, `viewport_width`, and - `viewport_height` metadata based on the current browser session. -- `properties` (object) - Custom metadata for signature-based baseline matching. - Reserved runtime fields stay pinned to the current browser session; explicit - viewport fields are still allowed when a test intentionally needs a custom - signature. +- Vizzly adds `browser` and `url` properties from the current browser session. + User properties with the same names take precedence. +- `properties` (object) - Any user metadata used for baseline matching, such as + theme, locale, or state. Vizzly reads dimensions from the captured image. - `threshold` (number) - Vizzly diff sensitivity threshold. When omitted, the Vizzly server configuration is used. - `minClusterSize` (number) - Ignore connected diff clusters smaller than this size - `fullPage` (boolean) - Capture full scrollable page instead of viewport. This applies to page targets; locator targets stay element-sized. diff --git a/clients/vitest/src/index.js b/clients/vitest/src/index.js index 5cbed194..3dd77ff7 100644 --- a/clients/vitest/src/index.js +++ b/clients/vitest/src/index.js @@ -40,7 +40,7 @@ * await expect(page).toMatchScreenshot('hero.png', { * properties: { * theme: 'dark', - * viewport: { width: 1920, height: 1080 } + * locale: 'en-US' * }, * threshold: 5 * }) diff --git a/clients/vitest/src/setup.js b/clients/vitest/src/setup.js index edb23218..bf658a16 100644 --- a/clients/vitest/src/setup.js +++ b/clients/vitest/src/setup.js @@ -10,38 +10,13 @@ export function buildScreenshotProperties( context = {} ) { let customProperties = options.properties ?? {}; - let customViewport = customProperties.viewport; - let customViewportWidth = customProperties.viewport_width; - let customViewportHeight = customProperties.viewport_height; - let viewportWidth = context.viewport?.width; - let viewportHeight = context.viewport?.height; - let properties = { - ...customProperties, + return { framework: 'vitest', vitest: true, url: locationHref, browser: context.browser || detectBrowser(), + ...customProperties, }; - - if (Number.isFinite(viewportWidth) && Number.isFinite(viewportHeight)) { - properties.viewport = { width: viewportWidth, height: viewportHeight }; - properties.viewport_width = viewportWidth; - properties.viewport_height = viewportHeight; - } - - if (customViewport !== undefined) { - properties.viewport = customViewport; - } - - if (customViewportWidth !== undefined) { - properties.viewport_width = customViewportWidth; - } - - if (customViewportHeight !== undefined) { - properties.viewport_height = customViewportHeight; - } - - return properties; } export function detectBrowser(userAgent = globalThis.navigator?.userAgent) { @@ -144,10 +119,6 @@ async function toMatchScreenshot(element, name, options = {}) { // Prepare properties let properties = buildScreenshotProperties(options, window.location.href, { element: isElementScreenshotTarget(element, page), - viewport: { - width: window.innerWidth, - height: window.innerHeight, - }, }); let isElement = isElementScreenshotTarget(element, page); diff --git a/clients/vitest/tests/vitest-plugin.spec.js b/clients/vitest/tests/vitest-plugin.spec.js index 803b8822..7ea3a7fa 100644 --- a/clients/vitest/tests/vitest-plugin.spec.js +++ b/clients/vitest/tests/vitest-plugin.spec.js @@ -168,7 +168,7 @@ describe('Vitest Plugin Integration', () => { }); describe('Custom Matcher', () => { - it('keeps comparison options out of screenshot properties', () => { + it('does not generate viewport properties from browser dimensions', () => { let properties = buildScreenshotProperties( { properties: { theme: 'dark' }, @@ -182,14 +182,11 @@ describe('Vitest Plugin Integration', () => { vitest: true, url: 'http://localhost/component', browser: 'unknown', - viewport: { width: 1920, height: 1080 }, - viewport_width: 1920, - viewport_height: 1080, theme: 'dark', }); }); - it('keeps reserved screenshot metadata stable while preserving viewport overrides', () => { + it('preserves viewport keys when the user explicitly supplies them', () => { let properties = buildScreenshotProperties( { properties: { @@ -207,10 +204,10 @@ describe('Vitest Plugin Integration', () => { ); expect(properties).toMatchObject({ - framework: 'vitest', - vitest: true, - url: 'http://localhost/component', - browser: 'unknown', + framework: 'custom-framework', + vitest: false, + url: 'http://evil.example', + browser: 'webkit', viewport: { width: 375, height: 667 }, viewport_width: 375, viewport_height: 667, @@ -282,7 +279,7 @@ describe('Vitest Plugin Integration', () => { }); }); - it('lets explicit user viewport properties override detected viewport metadata', () => { + it('does not reinterpret explicit user viewport properties', () => { let properties = buildScreenshotProperties( { properties: { diff --git a/src/api/core.js b/src/api/core.js index 67b65d6c..3d61d906 100644 --- a/src/api/core.js +++ b/src/api/core.js @@ -77,7 +77,7 @@ export function buildRequestHeaders({ * Build payload for screenshot upload or SHA resolution * @param {string} name - Screenshot name * @param {Buffer|null} buffer - Image data, or null when resolving by SHA - * @param {Object} metadata - Screenshot metadata (viewport, browser, etc.) + * @param {Object} metadata - User screenshot properties * @param {string|null} sha256 - Pre-computed SHA256 hash (optional) * @returns {Object} Screenshot upload payload */ @@ -109,7 +109,7 @@ export function buildScreenshotPayload( /** * Build payload for server-side screenshot resolution without sending image bytes * @param {string} name - Screenshot name - * @param {Object} metadata - Screenshot metadata (viewport, browser, etc.) + * @param {Object} metadata - User screenshot properties * @param {string} sha256 - Pre-computed SHA256 hash * @returns {Object} Screenshot resolution payload */ @@ -224,9 +224,6 @@ export function buildScreenshotCheckObject(sha256, name, metadata = {}) { return { sha256, name, - browser: meta.browser || 'chrome', - viewport_width: meta.viewport?.width || meta.viewport_width || 1920, - viewport_height: meta.viewport?.height || meta.viewport_height || 1080, properties, }; } diff --git a/src/client/index.js b/src/client/index.js index 28cd0ae6..ac920f5b 100644 --- a/src/client/index.js +++ b/src/client/index.js @@ -219,10 +219,6 @@ function createSimpleClient(serverUrl, clientOptions = {}) { let requestTimeout = normalizedOptions.requestTimeout || DEFAULT_TIMEOUT_MS; - for (let warning of normalizedOptions.warnings) { - console.warn(`[vizzly] ${warning.message}`); - } - try { // If it's a string, assume it's a file path and send directly // Otherwise it's a Buffer, so convert to base64 @@ -241,10 +237,6 @@ function createSimpleClient(serverUrl, clientOptions = {}) { screenshotData, getScreenshotOptionsPayload(normalizedOptions) ); - if (normalizedOptions.warnings.length > 0) { - screenshotData.warnings = normalizedOptions.warnings; - } - let httpStart = Date.now(); let { status, json } = await httpPost( `${serverUrl}/screenshot`, diff --git a/src/sdk/index.js b/src/sdk/index.js index ce1f5dba..63636fab 100644 --- a/src/sdk/index.js +++ b/src/sdk/index.js @@ -323,12 +323,6 @@ export class VizzlySDK extends EventEmitter { let buffer = resolveImageBuffer(imageBuffer, 'screenshot'); let normalizedOptions = normalizeScreenshotOptions(options); - for (let warning of normalizedOptions.warnings) { - output.warn(warning.message, { - code: warning.code, - option: warning.option, - }); - } // Generate or use provided build ID let buildId = normalizedOptions.buildId || this.currentBuildId || 'default'; @@ -348,10 +342,6 @@ export class VizzlySDK extends EventEmitter { screenshotData, getScreenshotOptionsPayload(normalizedOptions) ); - if (normalizedOptions.warnings.length > 0) { - screenshotData.warnings = normalizedOptions.warnings; - } - // POST to the local screenshot server let serverUrl = `http://localhost:${this.config.server?.port || 3000}`; let fetchFn = this.services.fetch || fetch; diff --git a/src/server/handlers/api-handler.js b/src/server/handlers/api-handler.js index 8798b1bb..b819903f 100644 --- a/src/server/handlers/api-handler.js +++ b/src/server/handlers/api-handler.js @@ -97,7 +97,7 @@ export let createApiHandler = ( deviceScaleFactor: normalizedOptions.deviceScaleFactor, selector: normalizedOptions.selector, }; - warnings = [...(warnings || []), ...normalizedOptions.warnings]; + warnings = warnings || []; let capture = { name, status: 'pending' }; captures.push(capture); let inputType = ['base64', 'file-path'].includes(type) diff --git a/src/server/handlers/tdd-handler.js b/src/server/handlers/tdd-handler.js index d3c8dbb9..8fad6af8 100644 --- a/src/server/handlers/tdd-handler.js +++ b/src/server/handlers/tdd-handler.js @@ -10,7 +10,6 @@ import { getDimensionsSync as defaultGetDimensionsSync } from '@vizzly-testing/h import { TddService as DefaultTddService } from '../../tdd/tdd-service.js'; import { detectImageInputType as defaultDetectImageInputType } from '../../utils/image-input-detector.js'; import * as defaultOutput from '../../utils/output.js'; -import { normalizeScreenshotOptions } from '../../utils/screenshot-options.js'; import { safePath as defaultSafePath, sanitizeScreenshotName as defaultSanitizeScreenshotName, @@ -18,43 +17,19 @@ import { } from '../../utils/security.js'; /** - * Unwrap double-nested properties if needed - * Client SDK wraps options in properties field, so we may get { properties: { properties: {...} } } + * Build the flat internal comparison shape. Width and height always come from + * the captured image. The untouched user bag remains available in metadata. */ -export const unwrapProperties = properties => { - if (!properties) return {}; - if (properties.properties && typeof properties.properties === 'object') { - // Merge top-level properties with nested properties - let unwrapped = { - ...properties, - ...properties.properties, - }; - // Remove the nested properties field to avoid confusion - delete unwrapped.properties; - return unwrapped; - } - return properties; -}; - -/** - * Extract properties to top-level format matching cloud API - * Normalizes viewport to viewport_width/height, ensures browser is set - */ -export const extractProperties = validatedProperties => { +export const extractProperties = ( + validatedProperties, + imageDimensions = {} +) => { if (!validatedProperties) return {}; return { ...validatedProperties, - // Normalize viewport to top-level viewport_width/height (cloud format) - viewport_width: - validatedProperties.viewport?.width ?? - validatedProperties.viewport_width ?? - null, - viewport_height: - validatedProperties.viewport?.height ?? - validatedProperties.viewport_height ?? - null, + viewport_width: imageDimensions.width ?? null, + viewport_height: imageDimensions.height ?? null, browser: validatedProperties.browser ?? null, - // Preserve nested structure in metadata for backward compatibility metadata: validatedProperties, }; }; @@ -400,20 +375,10 @@ export const createTddHandler = ( }; } - // Preserve the old nested wrapper for older clients, then filter user - // properties before local comparison/report serialization. - let unwrappedProperties = unwrapProperties(properties); - let normalizedProperties = normalizeScreenshotOptions({ - properties: unwrappedProperties, - }); - warnings = [...(warnings || []), ...normalizedProperties.warnings]; - - // Validate and sanitize properties + // Validate user metadata without unwrapping nested keys. let validatedProperties; try { - validatedProperties = validateScreenshotProperties( - normalizedProperties.properties - ); + validatedProperties = validateScreenshotProperties(properties); } catch (error) { return { statusCode: 400, @@ -425,9 +390,6 @@ export const createTddHandler = ( }; } - // Extract ALL properties to top-level (matching cloud API behavior) - const extractedProperties = extractProperties(validatedProperties); - // Support both base64 encoded images and file paths // Vitest browser mode returns file paths, so we need to handle both // Use explicit type from client if provided (fast path), otherwise detect (slow path) @@ -490,23 +452,16 @@ export const createTddHandler = ( }; } - // Auto-detect image dimensions if viewport not provided - if ( - !extractedProperties.viewport_width || - !extractedProperties.viewport_height - ) { - try { - const dimensions = getDimensionsSync(imageBuffer); - if (!extractedProperties.viewport_width) { - extractedProperties.viewport_width = dimensions.width; - } - if (!extractedProperties.viewport_height) { - extractedProperties.viewport_height = dimensions.height; - } - } catch { - // Dimensions will use defaults - } + let imageDimensions = {}; + try { + imageDimensions = getDimensionsSync(imageBuffer); + } catch { + // Invalid images are handled by the comparison service. } + let extractedProperties = extractProperties( + validatedProperties, + imageDimensions + ); // Use the sanitized name as-is (no modification with browser/viewport) // Baseline matching uses signature logic (name + viewport_width + browser) diff --git a/src/server/routers/screenshot.js b/src/server/routers/screenshot.js index de0b6977..79109922 100644 --- a/src/server/routers/screenshot.js +++ b/src/server/routers/screenshot.js @@ -66,7 +66,7 @@ export function createScreenshotRouter({ screenshotHandler, defaultBuildId }) { image, normalizedOptions.properties, type, - [...(warnings || []), ...normalizedOptions.warnings], + warnings || [], { threshold: normalizedOptions.threshold, minClusterSize: normalizedOptions.minClusterSize, diff --git a/src/types/client.d.ts b/src/types/client.d.ts index 2657274a..f0c2d581 100644 --- a/src/types/client.d.ts +++ b/src/types/client.d.ts @@ -69,16 +69,17 @@ export interface ScreenshotResult { * @example * // With properties and comparison settings * await vizzlyScreenshot('checkout-form', screenshot, { - * properties: { browser: 'chrome', viewport: { width: 1920, height: 1080 } }, + * properties: { browser: 'chrome', theme: 'dark', locale: 'en-US' }, * threshold: 5, * minClusterSize: 10, * fullPage: true, * requestTimeout: 5000 * }); * - * `properties` is the user metadata bag. Comparison options are normalized - * into the server metadata payload, while `requestTimeout` stays on the - * client request and `buildId` only routes the screenshot to a build. + * `properties` is the user metadata bag and every key is preserved. Vizzly + * reads width and height from the captured image. Comparison options are sent + * separately, while `requestTimeout` stays on the client request and `buildId` + * only routes the screenshot to a build. */ export function vizzlyScreenshot( name: string, diff --git a/src/uploader/core.js b/src/uploader/core.js index 77ea4fcd..81cb5d53 100644 --- a/src/uploader/core.js +++ b/src/uploader/core.js @@ -180,12 +180,11 @@ export function buildFileMetadata(filePath, buffer) { * @returns {Object} Screenshot format for SHA check */ export function fileToScreenshotFormat(file) { + let browser = extractBrowserFromFilename(file.filename); return { sha256: file.sha256, name: file.filename.replace(/\.png$/, ''), - browser: extractBrowserFromFilename(file.filename) || 'chrome', - viewport_width: 1920, - viewport_height: 1080, + properties: browser ? { browser } : {}, }; } diff --git a/src/utils/screenshot-options.js b/src/utils/screenshot-options.js index dbd60d93..7725aba4 100644 --- a/src/utils/screenshot-options.js +++ b/src/utils/screenshot-options.js @@ -1,86 +1,3 @@ -/** - * Screenshot option names that are part of the SDK/config contract, not the - * user's arbitrary metadata bag. - */ -export let RESERVED_PROPERTY_OPTIONS = Object.freeze({ - threshold: { - message: - 'Move "threshold" out of properties; properties is only for user metadata.', - }, - minClusterSize: { - message: - 'Move "minClusterSize" out of properties; properties is only for user metadata.', - }, - min_cluster_size: { - message: - 'Move "min_cluster_size" out of properties; properties is only for user metadata.', - }, - fullPage: { - message: - 'Move "fullPage" out of properties; properties is only for user metadata.', - }, - full_page: { - message: - 'Move "full_page" out of properties; properties is only for user metadata.', - }, - captureMode: { - message: - 'Move "captureMode" out of properties; properties is only for user metadata.', - }, - capture_mode: { - message: - 'Move "capture_mode" out of properties; properties is only for user metadata.', - }, - deviceScaleFactor: { - message: - 'Move "deviceScaleFactor" out of properties; properties is only for user metadata.', - }, - device_scale_factor: { - message: - 'Move "device_scale_factor" out of properties; properties is only for user metadata.', - }, - pixelRatio: { - message: - 'Move "pixelRatio" out of properties; properties is only for user metadata.', - }, - dpr: { - message: - 'Move "dpr" out of properties; properties is only for user metadata.', - }, - selector: { - message: - 'Move "selector" out of properties; properties is only for user metadata.', - }, - component: { - message: - 'Move "component" out of properties; properties is only for user metadata.', - }, - elementSelector: { - message: - 'Move "elementSelector" out of properties; properties is only for user metadata.', - }, - element_selector: { - message: - 'Move "element_selector" out of properties; properties is only for user metadata.', - }, - buildId: { - message: - 'Move "buildId" out of properties; properties is only for user metadata.', - }, - build_id: { - message: - 'Move "build_id" out of properties; properties is only for user metadata.', - }, - requestTimeout: { - message: - 'Move "requestTimeout" out of properties; properties is only for user metadata.', - }, - request_timeout: { - message: - 'Move "request_timeout" out of properties; properties is only for user metadata.', - }, -}); - export let SCREENSHOT_OPTION_NAMES = Object.freeze([ 'threshold', 'minClusterSize', @@ -98,18 +15,8 @@ export function getScreenshotOptionsPayload(options = {}) { ); } -function createReservedPropertyWarning(option) { - return { - code: 'reserved-property-option', - option, - message: RESERVED_PROPERTY_OPTIONS[option].message, - }; -} - /** - * Normalize screenshot SDK options into the local and cloud upload payload. - * Reserved names found inside properties are discarded instead of being promoted - * into options. + * Keep user properties separate from the Vizzly options sent beside them. */ export function normalizeScreenshotOptions(options = {}) { let { @@ -124,22 +31,11 @@ export function normalizeScreenshotOptions(options = {}) { selector, } = options; - let warnings = []; - let normalizedProperties = {}; let sourceProperties = properties && typeof properties === 'object' && !Array.isArray(properties) ? properties : {}; - for (let [key, value] of Object.entries(sourceProperties)) { - if (RESERVED_PROPERTY_OPTIONS[key]) { - warnings.push(createReservedPropertyWarning(key)); - continue; - } - - normalizedProperties[key] = value; - } - return { buildId, requestTimeout, @@ -149,8 +45,7 @@ export function normalizeScreenshotOptions(options = {}) { captureMode, deviceScaleFactor, selector, - properties: normalizedProperties, - warnings, + properties: { ...sourceProperties }, }; } diff --git a/src/utils/security.js b/src/utils/security.js index 2e778287..44956747 100644 --- a/src/utils/security.js +++ b/src/utils/security.js @@ -185,76 +185,56 @@ export function safePath(workingDir, ...pathSegments) { } /** - * Validates screenshot properties object for safe values - * @param {Object} properties - Properties to validate - * @returns {Object} Validated properties object + * Validate user metadata without interpreting names as Vizzly options or + * rewriting values. Rendering code must escape strings for its output context. + * Reject unsafe object keys and values that cannot be represented as JSON. + * + * @param {Object} [properties={}] - User screenshot metadata. + * @returns {Object} A copy preserving the supplied JSON values. + * @throws {Error} Metadata contains unsafe keys, cycles, or non-JSON values. */ export function validateScreenshotProperties(properties = {}) { - if (properties === null || typeof properties !== 'object') { + if ( + !properties || + typeof properties !== 'object' || + Array.isArray(properties) + ) { return {}; } - const validated = {}; - - // Validate common properties with safe constraints - if (properties.browser && typeof properties.browser === 'string') { - try { - // Extract browser name without version (e.g., "Chrome/139.0.7258.138" -> "Chrome") - const browserName = properties.browser.split('/')[0]; - validated.browser = sanitizeScreenshotName(browserName, 50); - } catch (error) { - // Skip invalid browser names, don't include them - output.warn( - `Invalid browser name '${properties.browser}': ${error.message}` - ); - } - } - - if (properties.viewport && typeof properties.viewport === 'object') { - const viewport = {}; + let ancestors = new Set(); + function copy(value) { if ( - typeof properties.viewport.width === 'number' && - properties.viewport.width > 0 && - properties.viewport.width <= 10000 + value === null || + typeof value === 'string' || + typeof value === 'boolean' ) { - viewport.width = Math.floor(properties.viewport.width); + return value; } - if ( - typeof properties.viewport.height === 'number' && - properties.viewport.height > 0 && - properties.viewport.height <= 10000 - ) { - viewport.height = Math.floor(properties.viewport.height); + if (typeof value === 'number' && Number.isFinite(value)) return value; + if (typeof value !== 'object' || ancestors.has(value)) { + throw new Error('Screenshot properties must contain JSON values'); } - if (Object.keys(viewport).length > 0) { - validated.viewport = viewport; - } - } - - // Allow other safe string properties but sanitize them - for (const [key, value] of Object.entries(properties)) { - if (key === 'browser' || key === 'viewport') continue; // Already handled - - if ( - typeof key === 'string' && - key.length <= 50 && - /^[a-zA-Z0-9_-]+$/.test(key) - ) { - if (typeof value === 'string' && value.length <= 200) { - // Preserve safe URL/query characters like '&' in metadata values. - // Rendering layers should escape for HTML instead of mutating payload data here. - validated[key] = value.replace(/[<>"']/g, ''); - } else if ( - typeof value === 'number' && - !Number.isNaN(value) && - Number.isFinite(value) + ancestors.add(value); + let result; + if (Array.isArray(value)) { + result = value.map(copy); + } else { + let entries = Object.entries(value); + if ( + entries.some(([key]) => + ['__proto__', 'constructor', 'prototype'].includes(key) + ) ) { - validated[key] = value; - } else if (typeof value === 'boolean') { - validated[key] = value; + throw new Error('Screenshot properties contain an unsafe key'); } + result = Object.fromEntries( + entries.map(([key, item]) => [key, copy(item)]) + ); } + ancestors.delete(value); + return result; } - return validated; + return copy(properties); } diff --git a/tests/api/core.test.js b/tests/api/core.test.js index 63d8309a..ec627452 100644 --- a/tests/api/core.test.js +++ b/tests/api/core.test.js @@ -473,48 +473,43 @@ describe('api/core', () => { }); describe('buildScreenshotCheckObject', () => { - it('builds check object with defaults', () => { + it('does not invent browser or image dimensions', () => { let result = buildScreenshotCheckObject('sha123', 'homepage'); assert.deepStrictEqual(result, { sha256: 'sha123', name: 'homepage', - browser: 'chrome', - viewport_width: 1920, - viewport_height: 1080, properties: {}, }); }); - it('uses metadata values when provided', () => { + it('keeps metadata inside properties', () => { let result = buildScreenshotCheckObject('sha123', 'homepage', { browser: 'firefox', viewport: { width: 1280, height: 720 }, }); - assert.strictEqual(result.browser, 'firefox'); - assert.strictEqual(result.viewport_width, 1280); - assert.strictEqual(result.viewport_height, 720); assert.deepStrictEqual(result.properties, { browser: 'firefox', viewport: { width: 1280, height: 720 }, }); }); - it('uses flat viewport_width/height from metadata', () => { + it('does not promote dimension-shaped properties', () => { let result = buildScreenshotCheckObject('sha123', 'homepage', { viewport_width: 800, viewport_height: 600, }); - assert.strictEqual(result.viewport_width, 800); - assert.strictEqual(result.viewport_height, 600); + assert.deepStrictEqual(result.properties, { + viewport_width: 800, + viewport_height: 600, + }); }); it('handles null metadata', () => { let result = buildScreenshotCheckObject('sha123', 'homepage', null); - assert.strictEqual(result.browser, 'chrome'); assert.deepStrictEqual(result.properties, {}); }); }); diff --git a/tests/cli/tdd-lifecycle.test.js b/tests/cli/tdd-lifecycle.test.js index a8837872..53f9a663 100644 --- a/tests/cli/tdd-lifecycle.test.js +++ b/tests/cli/tdd-lifecycle.test.js @@ -80,7 +80,7 @@ function parseSingleJson(stdout) { return parsed; } -function screenshotCommand(name) { +function screenshotCommand(name, properties = { browser: 'chromium' }) { let imagePath = join( process.cwd(), 'tests/reporter/fixtures/images/screenshots/homepage-desktop.png' @@ -88,7 +88,7 @@ function screenshotCommand(name) { let code = [ "let fs = await import('node:fs');", `let image = fs.readFileSync(${JSON.stringify(imagePath)}, 'base64');`, - `let payload = { name: ${JSON.stringify(name)}, image, type: 'base64', properties: { viewport_width: 1280, viewport_height: 720, browser: 'chromium' } };`, + `let payload = { name: ${JSON.stringify(name)}, image, type: 'base64', properties: ${JSON.stringify(properties)} };`, "let response = await fetch(process.env.VIZZLY_SERVER_URL + '/screenshot', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(payload) });", 'console.log(response.status, await response.text());', ].join(' '); @@ -255,6 +255,36 @@ describe('cli/tdd lifecycle', () => { assert.strictEqual(reportData.comparisons[0].status, 'passed'); }); + it('preserves user metadata in the saved local visual report', async () => { + let cwd = createWorkspace(); + let properties = { + browser: 'chromium', + threshold: 'user threshold', + component: '', + viewport: { width: 23, height: 17, label: 'user viewport' }, + properties: { nested: ['dark', null, true] }, + }; + let result = await runCLI( + [ + '--no-color', + 'tdd', + 'run', + screenshotCommand('metadata-cart', properties), + '--port', + String(await getFreePort()), + '--no-open', + ], + { cwd } + ); + assert.strictEqual(result.code, 0, JSON.stringify(result)); + let report = JSON.parse( + readFileSync(join(cwd, '.vizzly', 'report-data.json'), 'utf8') + ); + let comparison = report.comparisons[0]; + assert.deepStrictEqual(comparison.properties.metadata, properties); + assert.notStrictEqual(comparison.properties.viewport_width, 23); + }); + it('starts, reports, lists, and stops a daemon on an explicit port', async () => { let cwd = createWorkspace(); let port = await getFreePort(); diff --git a/tests/cli/upload-errors.test.js b/tests/cli/upload-errors.test.js index 47661a58..614d0f45 100644 --- a/tests/cli/upload-errors.test.js +++ b/tests/cli/upload-errors.test.js @@ -72,7 +72,7 @@ for (let scenario of [ let url = process.env.VIZZLY_SERVER_URL; async function post(path, body = {}) { if (path === '/screenshot') { - body = { ...body, properties: { theme: 'dark' }, threshold: 2, fullPage: true }; + body = { ...body, properties: { theme: 'dark', threshold: 'user threshold', properties: { component: 'Cart' } }, threshold: 2, fullPage: true }; } let response = await fetch(url + path, { method: 'POST', headers: { 'content-type': 'application/json' }, @@ -117,7 +117,11 @@ for (let scenario of [ for (let request of screenshotRequests) { assert.equal(request.threshold, 2); assert.equal(request.fullPage, true); - assert.deepEqual(request.properties, { theme: 'dark' }); + assert.deepEqual(request.properties, { + theme: 'dark', + threshold: 'user threshold', + properties: { component: 'Cart' }, + }); } assert.equal(finalized.length, 1); assert.equal(finalized[0].status, 'failed'); diff --git a/tests/sdk/client.test.js b/tests/sdk/client.test.js index 74003df4..2864320c 100644 --- a/tests/sdk/client.test.js +++ b/tests/sdk/client.test.js @@ -504,7 +504,7 @@ describe('client/index httpPost integration tests', () => { assert.deepStrictEqual(properties, { url: 'http://localhost:3000' }); }); - it('lets explicit comparison options override nested properties', async () => { + it('keeps explicit comparison options separate from user properties', async () => { await vizzlyScreenshot('test', Buffer.from('data'), { threshold: 0, minClusterSize: 2, @@ -517,19 +517,15 @@ describe('client/index httpPost integration tests', () => { assert.strictEqual(requests.length, 1); assert.strictEqual(requests[0].body.threshold, 0); assert.strictEqual(requests[0].body.minClusterSize, 2); - assert.deepStrictEqual(requests[0].body.properties, {}); - assert.deepStrictEqual( - requests[0].body.warnings.map(warning => warning.option), - ['threshold', 'minClusterSize'] - ); - assert.ok( - consoleWarnings.some(warning => - warning.includes('Move "threshold" out of properties') - ) - ); + assert.deepStrictEqual(requests[0].body.properties, { + threshold: 5, + minClusterSize: 10, + }); + assert.strictEqual(requests[0].body.warnings, undefined); + assert.deepStrictEqual(consoleWarnings, []); }); - it('does not promote reserved property options and returns warnings', async () => { + it('preserves option-shaped user properties without promoting them', async () => { await vizzlyScreenshot('test', Buffer.from('data'), { properties: { theme: 'dark', @@ -542,11 +538,13 @@ describe('client/index httpPost integration tests', () => { assert.strictEqual(requests.length, 1); assert.strictEqual(requests[0].body.buildId, undefined); assert.strictEqual(requests[0].body.threshold, undefined); - assert.deepStrictEqual(requests[0].body.properties, { theme: 'dark' }); - assert.deepStrictEqual( - requests[0].body.warnings.map(warning => warning.option), - ['buildId', 'requestTimeout', 'threshold'] - ); + assert.deepStrictEqual(requests[0].body.properties, { + theme: 'dark', + buildId: 'build-from-properties', + requestTimeout: 30_000, + threshold: 1, + }); + assert.strictEqual(requests[0].body.warnings, undefined); }); it('ignores arbitrary top-level metadata outside the user properties bag', async () => { diff --git a/tests/server/handlers/tdd-handler.test.js b/tests/server/handlers/tdd-handler.test.js index 381025d3..22c7601d 100644 --- a/tests/server/handlers/tdd-handler.test.js +++ b/tests/server/handlers/tdd-handler.test.js @@ -5,7 +5,6 @@ import { createTddHandler, extractProperties, groupComparisons, - unwrapProperties, } from '../../../src/server/handlers/tdd-handler.js'; /** @@ -118,91 +117,41 @@ function createMockDeps(overrides = {}) { } describe('server/handlers/tdd-handler', () => { - describe('unwrapProperties', () => { - it('returns empty object for null/undefined', () => { - assert.deepStrictEqual(unwrapProperties(null), {}); - assert.deepStrictEqual(unwrapProperties(undefined), {}); - }); - - it('returns properties as-is when not double-nested', () => { - let props = { browser: 'chrome', viewport: { width: 1920 } }; - assert.deepStrictEqual(unwrapProperties(props), props); - }); - - it('unwraps double-nested properties', () => { - let props = { - properties: { - browser: 'chrome', - viewport: { width: 1920, height: 1080 }, - }, - }; - - let result = unwrapProperties(props); - - assert.strictEqual(result.browser, 'chrome'); - assert.strictEqual(result.viewport.width, 1920); - assert.strictEqual(result.properties, undefined); - }); - - it('merges top-level and nested properties', () => { - let props = { - topLevel: 'value', - properties: { - browser: 'firefox', - }, - }; - - let result = unwrapProperties(props); - - assert.strictEqual(result.topLevel, 'value'); - assert.strictEqual(result.browser, 'firefox'); - assert.strictEqual(result.properties, undefined); - }); - }); - describe('extractProperties', () => { it('returns empty object for null/undefined', () => { assert.deepStrictEqual(extractProperties(null), {}); assert.deepStrictEqual(extractProperties(undefined), {}); }); - it('extracts viewport from nested structure', () => { + it('uses dimensions from the captured image', () => { let props = { browser: 'chrome', viewport: { width: 1920, height: 1080 }, }; - let result = extractProperties(props); + let result = extractProperties(props, { width: 800, height: 600 }); - assert.strictEqual(result.viewport_width, 1920); - assert.strictEqual(result.viewport_height, 1080); + assert.strictEqual(result.viewport_width, 800); + assert.strictEqual(result.viewport_height, 600); assert.strictEqual(result.browser, 'chrome'); + assert.deepStrictEqual(result.metadata.viewport, { + width: 1920, + height: 1080, + }); }); - it('uses top-level viewport_width/height if present', () => { - let props = { - viewport_width: 1280, - viewport_height: 720, - }; - - let result = extractProperties(props); - - assert.strictEqual(result.viewport_width, 1280); - assert.strictEqual(result.viewport_height, 720); - }); - - it('prefers nested viewport over top-level', () => { + it('does not treat user dimension properties as image dimensions', () => { let props = { - viewport: { width: 1920, height: 1080 }, viewport_width: 1280, viewport_height: 720, }; let result = extractProperties(props); - // Nested viewport.width takes precedence - assert.strictEqual(result.viewport_width, 1920); - assert.strictEqual(result.viewport_height, 1080); + assert.strictEqual(result.viewport_width, null); + assert.strictEqual(result.viewport_height, null); + assert.strictEqual(result.metadata.viewport_width, 1280); + assert.strictEqual(result.metadata.viewport_height, 720); }); it('sets null for missing values', () => { @@ -222,7 +171,7 @@ describe('server/handlers/tdd-handler', () => { }); describe('handleScreenshot', () => { - it('does not serialize operational fields as local comparison properties', async () => { + it('does not reinterpret user properties as screenshot options', async () => { let comparisonProperties; let deps = createMockDeps({ tddServiceOverrides: { @@ -261,10 +210,12 @@ describe('server/handlers/tdd-handler', () => { 'base64' ); - assert.deepStrictEqual(comparisonProperties.theme, 'dark'); - assert.strictEqual(comparisonProperties.threshold, undefined); + assert.strictEqual(comparisonProperties.threshold, 0.1); + assert.deepStrictEqual(comparisonProperties.properties, { + theme: 'dark', + minClusterSize: 4, + }); assert.strictEqual(comparisonProperties.minClusterSize, undefined); - assert.strictEqual(comparisonProperties.properties, undefined); }); }); diff --git a/tests/uploader/core.test.js b/tests/uploader/core.test.js index ceab9973..541f8ddb 100644 --- a/tests/uploader/core.test.js +++ b/tests/uploader/core.test.js @@ -264,20 +264,18 @@ describe('uploader/core', () => { assert.deepStrictEqual(result, { sha256: 'abc123', name: 'homepage-chrome', - browser: 'chrome', - viewport_width: 1920, - viewport_height: 1080, + properties: { browser: 'chrome' }, }); }); - it('defaults to chrome browser', () => { + it('does not invent a browser when the filename does not include one', () => { let file = { sha256: 'abc123', filename: 'screenshot.png', }; let result = fileToScreenshotFormat(file); - assert.strictEqual(result.browser, 'chrome'); + assert.deepStrictEqual(result.properties, {}); }); }); diff --git a/tests/utils/screenshot-options.test.js b/tests/utils/screenshot-options.test.js index 6a897120..c8c9ec52 100644 --- a/tests/utils/screenshot-options.test.js +++ b/tests/utils/screenshot-options.test.js @@ -22,7 +22,7 @@ describe('createScreenshotProperties', () => { assert.strictEqual(normalized.fullPage, true); }); - it('does not interpret reserved property names as options', () => { + it('keeps identically named properties separate from Vizzly options', () => { let normalized = normalizeScreenshotOptions({ threshold: 1, minClusterSize: 2, @@ -32,16 +32,15 @@ describe('createScreenshotProperties', () => { }, }); - assert.deepStrictEqual(normalized.properties, {}); + assert.deepStrictEqual(normalized.properties, { + threshold: 5, + minClusterSize: 10, + }); assert.strictEqual(normalized.threshold, 1); assert.strictEqual(normalized.minClusterSize, 2); - assert.deepStrictEqual( - normalized.warnings.map(warning => warning.option), - ['threshold', 'minClusterSize'] - ); }); - it('does not promote reserved properties into top-level options', () => { + it('preserves every property without promoting it into top-level options', () => { let normalized = normalizeScreenshotOptions({ properties: { theme: 'dark', @@ -54,23 +53,20 @@ describe('createScreenshotProperties', () => { }, }); - assert.deepStrictEqual(normalized.properties, { theme: 'dark' }); + assert.deepStrictEqual(normalized.properties, { + theme: 'dark', + threshold: 0.2, + minClusterSize: 5, + fullPage: true, + dpr: 2, + buildId: 'build-from-properties', + requestTimeout: 60_000, + }); assert.strictEqual(normalized.threshold, undefined); assert.strictEqual(normalized.minClusterSize, undefined); assert.strictEqual(normalized.fullPage, undefined); assert.strictEqual(normalized.buildId, undefined); assert.strictEqual(normalized.requestTimeout, undefined); - assert.deepStrictEqual( - normalized.warnings.map(warning => warning.option), - [ - 'threshold', - 'minClusterSize', - 'fullPage', - 'dpr', - 'buildId', - 'requestTimeout', - ] - ); }); it('ignores arbitrary top-level metadata outside the user properties bag', () => { diff --git a/tests/utils/security.test.js b/tests/utils/security.test.js index eed61ade..cd6f524c 100644 --- a/tests/utils/security.test.js +++ b/tests/utils/security.test.js @@ -197,126 +197,45 @@ describe('utils/security', () => { }); describe('validateScreenshotProperties', () => { - it('returns empty object for null input', () => { - let result = validateScreenshotProperties(null); - assert.deepStrictEqual(result, {}); - }); - - it('returns empty object for non-object input', () => { - let result = validateScreenshotProperties('string'); - assert.deepStrictEqual(result, {}); - }); - - it('returns empty object for empty properties', () => { - let result = validateScreenshotProperties({}); - assert.deepStrictEqual(result, {}); - }); - - it('validates browser name', () => { - let result = validateScreenshotProperties({ browser: 'Chrome/139.0' }); - assert.strictEqual(result.browser, 'Chrome'); - }); - - it('skips invalid browser names', () => { - let result = validateScreenshotProperties({ browser: '../etc' }); - assert.strictEqual(result.browser, undefined); - }); - - it('validates viewport dimensions', () => { - let result = validateScreenshotProperties({ - viewport: { width: 1920, height: 1080 }, - }); - assert.strictEqual(result.viewport.width, 1920); - assert.strictEqual(result.viewport.height, 1080); - }); - - it('rejects invalid viewport dimensions', () => { - let result = validateScreenshotProperties({ - viewport: { width: -100, height: 20000 }, - }); - assert.strictEqual(result.viewport, undefined); - }); - - it('floors viewport dimensions', () => { - let result = validateScreenshotProperties({ - viewport: { width: 1920.5, height: 1080.7 }, - }); - assert.strictEqual(result.viewport.width, 1920); - assert.strictEqual(result.viewport.height, 1080); - }); - - it('validates custom string properties', () => { - let result = validateScreenshotProperties({ - custom_key: 'value', - }); - assert.strictEqual(result.custom_key, 'value'); - }); - - it('preserves ampersands in safe string properties like URLs', () => { - let url = - 'http://localhost:6006/iframe.html?id=button--primary&viewMode=story'; - let result = validateScreenshotProperties({ - url, - }); - assert.strictEqual(result.url, url); - }); - - it('validates custom number properties', () => { - let result = validateScreenshotProperties({ - count: 42, - }); - assert.strictEqual(result.count, 42); - }); - - it('validates custom boolean properties', () => { - let result = validateScreenshotProperties({ - enabled: true, - }); - assert.strictEqual(result.enabled, true); - }); - - it('strips HTML entities from string values', () => { - let result = validateScreenshotProperties({ - desc: '', - }); - assert.ok(!result.desc.includes('<')); - assert.ok(!result.desc.includes('>')); - }); - - it('rejects invalid key names', () => { - let result = validateScreenshotProperties({ - 'invalid key!': 'value', - }); - assert.strictEqual(result['invalid key!'], undefined); - }); - - it('rejects overly long keys', () => { - let longKey = 'a'.repeat(100); - let result = validateScreenshotProperties({ - [longKey]: 'value', - }); - assert.strictEqual(result[longKey], undefined); - }); - - it('rejects overly long string values', () => { - let result = validateScreenshotProperties({ - key: 'a'.repeat(300), - }); - assert.strictEqual(result.key, undefined); - }); - - it('rejects NaN numbers', () => { - let result = validateScreenshotProperties({ - num: Number.NaN, - }); - assert.strictEqual(result.num, undefined); - }); - - it('rejects Infinity numbers', () => { - let result = validateScreenshotProperties({ - num: Number.POSITIVE_INFINITY, - }); - assert.strictEqual(result.num, undefined); + it('preserves nested JSON metadata without rewriting user values', () => { + let properties = { + browser: 'Chrome/139.0', + viewport: { width: 1920.5, height: 20000, label: 'custom' }, + 'component label': '