Unit test closed sink #40
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
dariomannu
requested changes
Oct 1, 2025
- Created BDD-style test suite following existing patterns - Tests cover ClosedSink function and Closed explicit sink - Includes dialog element closing functionality tests - Tests boolean attribute value handling (true/false) - Covers future/promise and observable sources - Includes edge cases and integration scenarios - Validates sink configuration structure - Uses custom MockDialogElement for HTMLDialogElement testing
TanmayRanaware
force-pushed
the
unit-test-closed-sink
branch
from
e6f0c0e to
feb17b4
Compare
added 6 commits
October 2, 2025 23:30
- Remove 'To Busy to read?, Check [TL;DR 📜](./QUICKSTART.md)' line from README - Delete QUICKSTART.md file completely - Clean up documentation structure
…iple - Split multiple actions into individual tests - Each test now has clear cause and effect - Better test isolation and debugging - More descriptive test names - Follows best practices for test organization
- Change 'closes dialog' to 'calls dialog.close()' in test names - Add comments explaining dialog.close() has no parameters - Clarify that there's no 'unclose' functionality - Each call to sink() results in dialog.close() being called regardless of input
- Add expectSinkBindingConfiguration helper function to test-support.ts - Replace repetitive sink configuration assertions with shared helper - Improve test maintainability and reduce code duplication - Helper can be reused across all sink tests
- Remove parameters from ClosedSink calls (sink doesn't take parameters) - Remove redundant test for source reference preservation - Remove unrelated swap-source.test.ts file from closed-sink PR - Simplify tests to focus on actual ClosedSink behavior - Clean up test structure and remove unnecessary complexity
This file exists in master but was accidentally removed from this branch. Restoring it to maintain consistency with master branch.
dariomannu
requested changes
Oct 3, 2025
- Remove redundant test for basic sink invocation - Remove redundant test for multiple calls - Remove redundant test for separate bindings - Keep only essential tests that add value - Focus on core functionality rather than exhaustive parameter testing
Contributor
|
Amazing, that's merged now! |
Contributor
Author
|
thenks @dariomannu for the great code reviweing. Liked the way you guided. Looking forward to solve more issues |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #31
Test coverage:
ClosedSink function and Closed explicit sink
Dialog element closing functionality
Boolean attribute values (true/false)
Promise and observable sources
Edge cases and integration scenarios