Skip to content

Add containerization extras tests #148

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

KushagraSikka
Copy link

@KushagraSikka KushagraSikka commented Jun 17, 2025

please verify, it requires Swift 6.2 to run, which aligns with the project's stated requirements.

📋 Test Coverage Created

  1. TestAsyncLock.swift (13 comprehensive tests):
  • Basic locking functionality
  • Sequential vs concurrent access patterns
  • Error handling and recovery
  • FIFO ordering and reentrancy prevention
  • Performance characteristics
  • Multiple lock instances
  • Complex return types and edge cases
  1. TestTimeout.swift (15 comprehensive tests):
  • Successful operations and quick operations
  • Timeout behavior and accuracy
  • Error propagation and concurrent timeouts
  • Task cancellation and cleanup
  • Performance and resource management

Edge cases (zero timeout, large timeout, consecutive timeouts)

KushagraSikka and others added 5 commits June 17, 2025 07:26
…tras - Enhanced IPv4Address documentation with comprehensive examples and parameter descriptions - Improved error messages in NetworkAddressError with actionable guidance - Added detailed documentation for CIDRAddress including usage examples and method descriptions - Expanded AsyncLock documentation with use cases and threading safety information - Enhanced Timeout documentation with implementation details and performance notes - Improved error message clarity in Timeout.swift by replacing generic fatalError These changes make the API more accessible to new contributors and provide better developer experience through clear error messages and comprehensive documentation.
…s - Added TestAsyncLock.swift with 13 test cases covering concurrent access, error handling, FIFO ordering, and performance - Added TestTimeout.swift with 15 test cases covering timeout behavior, error propagation, and edge cases - Tests ensure thread safety, proper cancellation, and accurate timeout behavior - Improves test coverage for ContainerizationExtras module - All tests use modern Swift Testing framework with async/await patterns
…Fixed all Sendable protocol conformance issues in test structs - Replaced throwing closures with non-throwing ones for Timeout.run compatibility - Used actors for thread-safe shared state in concurrent tests - Resolved generic type conflicts in AsyncLock performance tests - Added proper error handling patterns using Result types - Ensured all async operations use proper concurrency patterns
@dcantah
Copy link
Member

dcantah commented Jun 17, 2025

This is awesome! I'll take a look at this sometime between today and tomorrow. Thanks a ton!

group.addTask {
await lock.withLock { context in
await tracker.append("task1-start")
try? await Task.sleep(nanoseconds: 50_000_000) // 50ms
Copy link
Member

@dcantah dcantah Jun 18, 2025

Choose a reason for hiding this comment

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

Task.sleep has a for: constructor that takes in enums for different time intervals and should look much nicer:

try await Task.sleep(for: .milliseconds(50))

Copy link
Member

Choose a reason for hiding this comment

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

Same advice for the other sleeps

@dcantah dcantah requested a review from dkovba June 18, 2025 23:54
@dcantah
Copy link
Member

dcantah commented Jun 18, 2025

@dkovba Wanna look at the AsyncLock documentation updates here?

Copy link
Contributor

@dkovba dkovba left a comment

Choose a reason for hiding this comment

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

Thanks 👍

/// await lock.withLock { context in
/// // Async operations are safe within the lock
/// let processedName = await processResourceName(name)
/// resources.append(processedName)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add func appendResource(_ resource: String, context: AsyncLock.Context) to demonstrate how context is used? The required context parameter ensures that the function is called only inside a lock.

/// A context object provided to closures executed within the lock.
///
/// The context serves as proof that the code is executing within the lock's
/// critical section. While currently empty, it may be extended in the future
Copy link
Contributor

Choose a reason for hiding this comment

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

It is empty by design because this is sufficient to serve as proof that the code is executed within the lock.

@dkovba
Copy link
Contributor

dkovba commented Jun 19, 2025

Please run make fmt.

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.

3 participants