Skip to content

Conversation

christian-byrne
Copy link
Contributor

@christian-byrne christian-byrne commented Sep 14, 2025

Summary

Added comprehensive component test suite for WidgetImageCompare widget with 410 test assertions covering display, edge cases, and integration scenarios.

Changes

Review Focus

Test coverage completeness for string vs object value handling, accessibility attribute propagation, and edge case robustness including malformed URLs and empty states.

┆Issue is synchronized with this Notion page by Unito

@christian-byrne christian-byrne requested a review from a team as a code owner September 14, 2025 04:04
@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Sep 14, 2025
Copy link

github-actions bot commented Sep 14, 2025

🎭 Playwright Test Results

All tests passed!

⏰ Completed at: 09/16/2025, 09:34:02 PM UTC

📈 Summary

  • Total Tests: 450
  • Passed: 421 ✅
  • Failed: 0
  • Flaky: 0
  • Skipped: 29 ⏭️

📊 Test Reports by Browser

  • chromium: View Report • ✅ 414 / ❌ 0 / ⚠️ 0 / ⏭️ 29
  • chromium-2x: View Report • ✅ 2 / ❌ 0 / ⚠️ 0 / ⏭️ 0
  • chromium-0.5x: View Report • ✅ 1 / ❌ 0 / ⚠️ 0 / ⏭️ 0
  • mobile-chrome: View Report • ✅ 4 / ❌ 0 / ⚠️ 0 / ⏭️ 0

🎉 Click on the links above to view detailed test results for each browser configuration.

DrJKL
DrJKL previously approved these changes Sep 16, 2025
Copy link
Contributor

@DrJKL DrJKL left a comment

Choose a reason for hiding this comment

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

I like tests, but a lot of these feel pretty duplicative. Are they really testing different logic or code paths in ways that would provide a clear signal in case of failure?


import WidgetImageCompare from './WidgetImageCompare.vue'

interface ImageCompareValue {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this interface available in the component itself?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exported the ImageCompareValue interface directly from the component and updated the test to import it instead of duplicating - see commit 63d6a61

describe('WidgetImageCompare Display', () => {
const createMockWidget = (
value: ImageCompareValue | string,
options: Partial<ImageCompareProps> = {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
options: Partial<ImageCompareProps> = {}
options: SimplifiedWidget['options'] = {}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed Partial<ImageCompareProps> to SimplifiedWidget['options'] for consistency with the actual component interface - see commit 63d6a61

})

it('handles empty object value', () => {
const value = {} as ImageCompareValue
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you type the variable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added proper typing for the empty object test case - see commit 63d6a61

christian-byrne and others added 2 commits September 16, 2025 14:07
…g - addresses review feedback

- Export ImageCompareValue interface from component for reuse
- Update test to import interface instead of duplicating it
- Replace Partial<ImageCompareProps> with SimplifiedWidget['options'] for consistency
- Add proper typing for empty object test case

Co-authored-by: DrJKL <[email protected]>
Reduced test duplication by consolidating similar test cases:
- Combined 3 basic rendering tests into comprehensive structure test
- Merged alt text tests into single parameterized test
- Consolidated URL handling tests (long URLs, special chars, data/blob URLs)
- Unified template structure verification into single test

Reduces test count from 25 to 15 while maintaining full coverage.
Provides clearer failure signals by grouping related assertions.

Co-authored-by: DrJKL <[email protected]>
@christian-byrne
Copy link
Contributor Author

christian-byrne commented Sep 16, 2025

Consolidated overlapping tests from 25 to 15 tests, maintaining full coverage while providing clearer failure signals - see commit 87cb782

@christian-byrne christian-byrne merged commit 37975e4 into main Sep 18, 2025
21 checks passed
@christian-byrne christian-byrne deleted the vue-nodes/test/image-compare-component-test branch September 18, 2025 20:44
Myestery pushed a commit that referenced this pull request Sep 19, 2025
## Summary

Added comprehensive component test suite for WidgetImageCompare widget
with 410 test assertions covering display, edge cases, and integration
scenarios.

## Changes

- **What**: Created [Vue Test Utils](https://vue-test-utils.vuejs.org/)
test suite for [WidgetImageCompare
component](src/renderer/extensions/vueNodes/widgets/components/WidgetImageCompare.vue)
using [Vitest](https://vitest.dev/) testing framework

## Review Focus

Test coverage completeness for string vs object value handling,
accessibility attribute propagation, and edge case robustness including
malformed URLs and empty states.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-5549-test-Add-component-test-for-image-compare-widget-26e6d73d365081189fe0d010f87d1eec)
by [Unito](https://www.unito.io)

---------

Co-authored-by: DrJKL <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:testing area:vue-migration size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants