[ECO-5465] Enable disabled integration tests #55
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis change introduces significant refactoring to LiveMap and related types, centralizing logic and synchronization within internal state structures. It also makes the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant CoreSDK
participant Logger
participant InternalPlugin
participant PluginAPI
Client->>CoreSDK: publish(objectMessages)
CoreSDK->>Logger: log formatted objectMessages
CoreSDK->>InternalPlugin: sendObject(objectMessages, channel, client, pluginAPI)
InternalPlugin->>PluginAPI: dispatch sendObject on client internal queue
PluginAPI-->>InternalPlugin: sendObject result
InternalPlugin-->>CoreSDK: async result
CoreSDK-->>Client: async result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
temporarily deployed
to
staging/pull/55/AblyLiveObjects
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Tests/AblyLiveObjectsTests/Helpers/TestFactories.swift (1)
408-417: Consider making internalMapEntry data parameter optional for consistencyThe
internalMapEntryfactory method still requires non-optionalObjectDatawhilemapEntrynow accepts optionalObjectData?. This creates an inconsistency between the two factory methods that should ideally mirror each other.Consider updating the signature to match:
static func internalMapEntry( tombstonedAt: Date? = nil, timeserial: String? = "ts1", - data: ObjectData, + data: ObjectData?, ) -> InternalObjectsMapEntry {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
Sources/AblyLiveObjects/Internal/CoreSDK.swift(1 hunks)Sources/AblyLiveObjects/Internal/DefaultInternalPlugin.swift(1 hunks)Sources/AblyLiveObjects/Internal/InternalDefaultLiveCounter.swift(2 hunks)Sources/AblyLiveObjects/Internal/InternalDefaultLiveMap.swift(6 hunks)Sources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swift(2 hunks)Sources/AblyLiveObjects/Internal/InternalObjectsMapEntry.swift(1 hunks)Sources/AblyLiveObjects/Protocol/ObjectMessage.swift(4 hunks)Sources/AblyLiveObjects/Protocol/WireObjectMessage.swift(3 hunks)Sources/AblyLiveObjects/Public/ARTRealtimeChannel+Objects.swift(1 hunks)Sources/AblyLiveObjects/Utility/LoggingUtilities.swift(1 hunks)Tests/AblyLiveObjectsTests/Helpers/ClientHelper.swift(2 hunks)Tests/AblyLiveObjectsTests/Helpers/TestFactories.swift(1 hunks)Tests/AblyLiveObjectsTests/InternalDefaultLiveMapTests.swift(3 hunks)Tests/AblyLiveObjectsTests/JS Integration Tests/ObjectsIntegrationTests.swift(12 hunks)Tests/AblyLiveObjectsTests/WireObjectMessageTests.swift(5 hunks)ably-cocoa(1 hunks)
🧰 Additional context used
🧠 Learnings (10)
📚 Learning: applies to ablyliveobjects/**/!(*test|*tests).swift : when importing the ably module inside the ably...
Learnt from: CR
PR: ably/ably-cocoa-liveobjects-plugin#0
File: .cursor/rules/swift.mdc:0-0
Timestamp: 2025-07-29T08:07:47.875Z
Learning: Applies to AblyLiveObjects/**/!(*Test|*Tests).swift : When importing the Ably module inside the AblyLiveObjects library code (non-test code), use `import Ably`.
Applied to files:
Tests/AblyLiveObjectsTests/JS Integration Tests/ObjectsIntegrationTests.swiftSources/AblyLiveObjects/Internal/InternalObjectsMapEntry.swiftSources/AblyLiveObjects/Utility/LoggingUtilities.swiftTests/AblyLiveObjectsTests/Helpers/ClientHelper.swiftSources/AblyLiveObjects/Protocol/ObjectMessage.swiftSources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swiftSources/AblyLiveObjects/Internal/CoreSDK.swiftTests/AblyLiveObjectsTests/Helpers/TestFactories.swiftSources/AblyLiveObjects/Public/ARTRealtimeChannel+Objects.swiftTests/AblyLiveObjectsTests/InternalDefaultLiveMapTests.swiftTests/AblyLiveObjectsTests/WireObjectMessageTests.swiftSources/AblyLiveObjects/Internal/InternalDefaultLiveMap.swift
📚 Learning: applies to **/tests/**/*.swift : when you need to import the following modules in the tests, do so i...
Learnt from: CR
PR: ably/ably-cocoa-liveobjects-plugin#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-07-29T08:07:58.385Z
Learning: Applies to **/Tests/**/*.swift : When you need to import the following modules in the tests, do so in the following way: Ably: use `import Ably`; AblyLiveObjects: use `@testable import AblyLiveObjects`; AblyPlugin: use `import AblyPlugin`; do not do `internal import`.
Applied to files:
Tests/AblyLiveObjectsTests/JS Integration Tests/ObjectsIntegrationTests.swiftSources/AblyLiveObjects/Utility/LoggingUtilities.swiftTests/AblyLiveObjectsTests/Helpers/ClientHelper.swiftSources/AblyLiveObjects/Internal/CoreSDK.swiftTests/AblyLiveObjectsTests/InternalDefaultLiveMapTests.swiftTests/AblyLiveObjectsTests/WireObjectMessageTests.swift
📚 Learning: applies to ablyliveobjects/**/!(*test|*tests).swift : when importing the ablyplugin module inside th...
Learnt from: CR
PR: ably/ably-cocoa-liveobjects-plugin#0
File: .cursor/rules/swift.mdc:0-0
Timestamp: 2025-07-29T08:07:47.875Z
Learning: Applies to AblyLiveObjects/**/!(*Test|*Tests).swift : When importing the AblyPlugin module inside the AblyLiveObjects library code (non-test code), use `internal import AblyPlugin`.
Applied to files:
Tests/AblyLiveObjectsTests/JS Integration Tests/ObjectsIntegrationTests.swiftSources/AblyLiveObjects/Internal/InternalObjectsMapEntry.swiftSources/AblyLiveObjects/Utility/LoggingUtilities.swiftTests/AblyLiveObjectsTests/Helpers/ClientHelper.swiftSources/AblyLiveObjects/Internal/DefaultInternalPlugin.swiftSources/AblyLiveObjects/Internal/CoreSDK.swiftSources/AblyLiveObjects/Public/ARTRealtimeChannel+Objects.swiftTests/AblyLiveObjectsTests/InternalDefaultLiveMapTests.swiftTests/AblyLiveObjectsTests/WireObjectMessageTests.swiftSources/AblyLiveObjects/Internal/InternalDefaultLiveMap.swift
📚 Learning: applies to **/tests/**/*.swift : do not use `fatalerror` in response to a test expectation failure. ...
Learnt from: CR
PR: ably/ably-cocoa-liveobjects-plugin#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-07-29T08:08:00.588Z
Learning: Applies to **/Tests/**/*.swift : Do not use `fatalError` in response to a test expectation failure. Favour the usage of Swift Testing's `#require` macro.
Applied to files:
Tests/AblyLiveObjectsTests/JS Integration Tests/ObjectsIntegrationTests.swift
📚 Learning: applies to **/tests/**/*.swift : add comments that explain when some piece of test data is not impor...
Learnt from: CR
PR: ably/ably-cocoa-liveobjects-plugin#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-07-29T08:08:00.588Z
Learning: Applies to **/Tests/**/*.swift : Add comments that explain when some piece of test data is not important for the scenario being tested.
Applied to files:
Tests/AblyLiveObjectsTests/JS Integration Tests/ObjectsIntegrationTests.swiftTests/AblyLiveObjectsTests/Helpers/TestFactories.swiftTests/AblyLiveObjectsTests/InternalDefaultLiveMapTests.swiftTests/AblyLiveObjectsTests/WireObjectMessageTests.swift
📚 Learning: applies to **/tests/**/*.swift : when you need to pass a logger to internal components in the tests,...
Learnt from: CR
PR: ably/ably-cocoa-liveobjects-plugin#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-07-29T08:08:00.588Z
Learning: Applies to **/Tests/**/*.swift : When you need to pass a logger to internal components in the tests, pass `TestLogger()`.
Applied to files:
Tests/AblyLiveObjectsTests/Helpers/ClientHelper.swiftSources/AblyLiveObjects/Internal/CoreSDK.swiftSources/AblyLiveObjects/Public/ARTRealtimeChannel+Objects.swift
📚 Learning: applies to **/tests/**/*.swift : when writing tests, make sure to add comments that explain when som...
Learnt from: CR
PR: ably/ably-cocoa-liveobjects-plugin#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-07-29T08:07:58.385Z
Learning: Applies to **/Tests/**/*.swift : When writing tests, make sure to add comments that explain when some piece of test data is not important for the scenario being tested.
Applied to files:
Tests/AblyLiveObjectsTests/InternalDefaultLiveMapTests.swiftTests/AblyLiveObjectsTests/WireObjectMessageTests.swift
📚 Learning: applies to **/tests/**/*.swift : when creating `testsonly_` property declarations, do not write gene...
Learnt from: CR
PR: ably/ably-cocoa-liveobjects-plugin#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-07-29T08:07:58.385Z
Learning: Applies to **/Tests/**/*.swift : When creating `testsOnly_` property declarations, do not write generic comments of the form 'Test-only access to the private createOperationIsMerged property'; the meaning of these properties is already well understood.
Applied to files:
Tests/AblyLiveObjectsTests/InternalDefaultLiveMapTests.swiftTests/AblyLiveObjectsTests/WireObjectMessageTests.swift
📚 Learning: applies to **/tests/**/*.swift : when you need to unwrap an optional value in the tests, favour usin...
Learnt from: CR
PR: ably/ably-cocoa-liveobjects-plugin#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-07-29T08:08:00.588Z
Learning: Applies to **/Tests/**/*.swift : When you need to unwrap an optional value in the tests, favour using `#require` instead of `guard let`.
Applied to files:
Tests/AblyLiveObjectsTests/InternalDefaultLiveMapTests.swiftTests/AblyLiveObjectsTests/WireObjectMessageTests.swift
📚 Learning: applies to **/*.swift : when writing jsonvalue or wirevalue types, favour using the literal syntax e...
Learnt from: CR
PR: ably/ably-cocoa-liveobjects-plugin#0
File: .cursor/rules/swift.mdc:0-0
Timestamp: 2025-07-29T08:07:47.875Z
Learning: Applies to **/*.swift : When writing JSONValue or WireValue types, favour using the literal syntax enabled by their conformance to the `ExpressibleBy*Literal` protocols where possible.
Applied to files:
Tests/AblyLiveObjectsTests/WireObjectMessageTests.swiftSources/AblyLiveObjects/Protocol/WireObjectMessage.swift
🧬 Code Graph Analysis (5)
Tests/AblyLiveObjectsTests/JS Integration Tests/ObjectsIntegrationTests.swift (3)
Sources/AblyLiveObjects/Public/PublicTypes.swift (1)
updates(425-437)Tests/AblyLiveObjectsTests/JS Integration Tests/ObjectsHelper.swift (1)
createAndSetOnMap(380-397)Sources/AblyLiveObjects/Internal/InternalDefaultLiveMap.swift (2)
get(112-114)get(806-822)
Sources/AblyLiveObjects/Internal/InternalDefaultLiveCounter.swift (1)
Sources/AblyLiveObjects/Internal/CoreSDK.swift (1)
validateChannelState(62-74)
Sources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swift (2)
Tests/AblyLiveObjectsTests/Helpers/ClientHelper.swift (1)
log(55-58)Sources/AblyLiveObjects/Utility/LoggingUtilities.swift (1)
formatObjectMessagesForLogging(7-13)
Sources/AblyLiveObjects/Internal/DefaultInternalPlugin.swift (1)
Sources/AblyLiveObjects/Utility/InternalError.swift (2)
toInternalError(28-30)toInternalError(34-36)
Tests/AblyLiveObjectsTests/InternalDefaultLiveMapTests.swift (1)
Tests/AblyLiveObjectsTests/ObjectMessageTests.swift (3)
number(133-145)number(229-241)number(326-338)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Xcode, iOS (Xcode 16.3)
- GitHub Check: Xcode, tvOS (Xcode 16.3)
- GitHub Check: Xcode, macOS (Xcode 16.3)
- GitHub Check: Generate code coverage
- GitHub Check: Xcode, iOS (Xcode 16.3)
🔇 Additional comments (34)
Sources/AblyLiveObjects/Utility/LoggingUtilities.swift (3)
1-1: LGTM!Appropriate import for the utility functionality.
3-6: LGTM!The enum structure and documentation follow Swift conventions well. The internal access level is appropriate for utility code within the module.
7-13: LGTM! Clean and efficient implementation.The method implementation is well-structured with:
- Modern Swift generics syntax using
some CustomDebugStringConvertible- Proper empty array handling
- Clean multi-line formatting with consistent indentation
- Efficient string concatenation approach
This utility will provide consistent, readable log formatting across the codebase.
ably-cocoa (1)
1-1: Confirm the submodule SHA is pushed and reachableI ran
git ls-remoteagainst https://github.com/ably/ably-cocoa.git forbc19368559fe1b860d18f375e421e8fd2526572fbut saw no output (sincels-remoteonly shows refs). Please manually verify that:
- The commit
bc19368559fe1b860d18f375e421e8fd2526572fis present on theably/ably-cocoaremote (e.g. included in a branch or tag).- All CI mirrors and forks have this commit (so
git submodule update --init --recursivepicks it up).- Any dependency managers (SPM/Carthage/CocoaPods) that vendor Ably Cocoa reference this exact SHA.
Sources/AblyLiveObjects/Internal/InternalObjectsMapEntry.swift (1)
12-12: LGTM: Clean transition to optional data propertyThe change from
ObjectDatatoObjectData?is consistent with the broader refactoring to support optional data in map entries. This allows for proper handling of tombstoned or empty entries.Sources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swift (2)
392-392: LGTM: Improved logging format for better debuggingThe use of
LoggingUtilities.formatObjectMessagesForLoggingwill produce much more readable log output compared to direct array interpolation, especially when dealing with multiple object messages.
492-492: LGTM: Consistent logging improvementSame formatting improvement as above, ensuring consistent and readable log output across both sync and regular object message handling.
Tests/AblyLiveObjectsTests/Helpers/TestFactories.swift (1)
396-396: Good alignment with optional data changesThe parameter change to optional
ObjectData?correctly aligns with the broader refactoring to make map entry data optional throughout the codebase.Sources/AblyLiveObjects/Public/ARTRealtimeChannel+Objects.swift (1)
15-22: LGTM: Improved initialization order and logger consistencyMoving the logger initialization before
coreSDKcreation allows the same logger instance to be passed toDefaultCoreSDKand ensures consistent logging behavior across all components. This is a clean refactoring that maintains functionality while improving the initialization pattern.Tests/AblyLiveObjectsTests/WireObjectMessageTests.swift (4)
183-183: LGTM: Correct optional chaining for optional data propertyThe addition of optional chaining (
?.data?.string) correctly handles the fact that thedataproperty in wire object map entries is now optional, preventing potential runtime crashes.
285-285: LGTM: Consistent optional handlingSame correct pattern of optional chaining to safely access the optional
dataproperty.
491-491: LGTM: Proper optional chaining in map entry testsBoth assertions correctly use optional chaining to handle the optional
dataproperty in wire map entries.Also applies to: 494-494
587-587: LGTM: Complete optional handling in wire map entry testsThe test assertions correctly handle the optional
dataproperty with proper optional chaining, ensuring tests remain robust with the new optional data semantics.Also applies to: 596-596
Tests/AblyLiveObjectsTests/InternalDefaultLiveMapTests.swift (3)
537-538: LGTM! Correct adaptation to optional data property.The test correctly uses optional chaining to access the
dataproperty fields, reflecting the change from non-optional to optionalObjectsMapEntry.data.
720-720: LGTM! Correct nil check for optional data property.The assertion correctly verifies that
dataisnilfor aMAP_REMOVEoperation, which aligns with the specification requirement "Set ObjectsMapEntry.data to undefined/null" (RTLM8a2a).
749-749: LGTM! Consistent nil check for new entry data.The test correctly verifies that newly created entries from
MAP_REMOVEoperations havenildata, which is consistent with the specification requirement (RTLM8b1).Sources/AblyLiveObjects/Internal/CoreSDK.swift (4)
19-19: LGTM! Good addition of logging capability.Adding the logger property enables structured logging within the CoreSDK, which is beneficial for debugging and monitoring.
24-25: LGTM! Constructor properly updated for logger injection.The constructor correctly accepts and stores the logger parameter, maintaining consistency with the dependency injection pattern used throughout the codebase.
Also applies to: 30-30
36-36: LGTM! Valuable debug logging added.The debug log statement using
LoggingUtilities.formatObjectMessagesForLoggingwill provide helpful visibility into object message publishing for debugging purposes.
42-42: LGTM! Client parameter addition aligns with DefaultInternalPlugin changes.Adding the
clientparameter to thesendObjectcall is consistent with the updated method signature inDefaultInternalPlugin.swift, enabling proper queue dispatching.Sources/AblyLiveObjects/Internal/InternalDefaultLiveCounter.swift (2)
83-83: LGTM! Clean delegation to internal state method.The refactoring properly delegates the validation and value retrieval to the
MutableState, improving encapsulation and consistency with similar patterns in the codebase.
417-423: LGTM! Well-implemented internal state method.The new
value(coreSDK:)method inMutableStatecorrectly:
- Validates channel state per RTLC5b specification
- Returns the counter data per RTLC5c specification
- Maintains the same error handling behavior as the original implementation
This refactoring centralizes state-related logic within the
MutableStatestruct, which is a good architectural improvement.Sources/AblyLiveObjects/Internal/DefaultInternalPlugin.swift (2)
131-131: LGTM! Client parameter addition enables proper queue dispatching.Adding the
clientparameter allows the method to obtain the correct internal queue for thread-safe message sending, which is essential for proper SDK operation.
137-151: LGTM! Proper async dispatch implementation.The implementation correctly:
- Obtains the client's internal queue via
pluginAPI.internalQueue(for: client)- Dispatches the
sendObjectcall asynchronously on that queue- Properly handles both success and error cases in the completion handler
- Uses the existing continuation pattern for async/await integration
This ensures thread-safe message sending while maintaining the existing async interface.
Sources/AblyLiveObjects/Protocol/WireObjectMessage.swift (2)
562-604: LGTM! Well-structured debug descriptionsThe
CustomDebugStringConvertibleimplementations provide comprehensive and consistent debug representations that will enhance logging and debugging capabilities.
434-434: No issues found with optionaldatausage
All direct accesses toWireObjectsMapEntry.datanow use safe optional handling, and initializations accept the new optional parameter correctly:
- In
Sources/AblyLiveObjects/Protocol/ObjectMessage.swift,datais unwrapped viaif let wireObjectData = wireObjectsMapEntry.data { … }.- All tests in
WireObjectMessageTests.swiftuse optional chaining (entry.data?.string) or pass aWireObjectDataliteral into the now-optional parameter without forcing unwraps.No further changes are required.
Sources/AblyLiveObjects/Protocol/ObjectMessage.swift (2)
74-74: LGTM! Consistent optional data handlingThe changes correctly implement optional data handling for
ObjectsMapEntry, maintaining consistency with the wire format changes inWireObjectsMapEntry.Also applies to: 389-393, 405-405
476-603: Excellent debug string implementationsThe
CustomDebugStringConvertibleconformances are well-designed:
- Consistent formatting across all types
- Smart choice to display byte count instead of raw data
- Clear hierarchical representation for nested structures
Tests/AblyLiveObjectsTests/JS Integration Tests/ObjectsIntegrationTests.swift (4)
91-97: Excellent refactoring to use AsyncStreamThe simplified implementation using
AsyncStream.firstis much cleaner and more idiomatic than the previous subscription-based approach. This removes manual continuation management and makes the code more maintainable.
160-174: Good test setup improvementsCreating
ClientHelper.PartialClientOptionsonce and modifying it for different test scenarios reduces duplication. ThelogIdentifieraddition will help distinguish client logs during test runs.
304-312: Consistent AsyncStream usage patternThe tests correctly use the
updates()method to obtain AsyncStreams and pass them to the wait helpers. The pattern of creating multiple streams for concurrent update monitoring is appropriate.Also applies to: 327-339
296-296: Great to see previously disabled tests now enabledThe enabling of these integration tests indicates that the required features have been implemented, which aligns with the PR's objective of "[ECO-5465] Enable disabled integration tests".
Also applies to: 371-371, 513-513, 569-569, 624-624
Sources/AblyLiveObjects/Internal/InternalDefaultLiveMap.swift (2)
616-617: LGTM! Proper handling of optional data propertyThe changes correctly handle the new optionality of the
dataproperty:
- Optional parameter type in method signature
- Safe optional chaining when accessing
objectId- Setting
datatonilfor tombstoned entries is cleaner than using empty ObjectDataAlso applies to: 645-645, 682-683, 691-691
805-935: Well-structured refactoring with proper RTLM complianceThe refactored methods correctly implement the RTLM specifications:
- Proper channel state validation (RTLM5c, RTLM10c, RTLM11c)
- Correct tombstone handling at multiple levels (map, entry, and referenced objects)
- Systematic value conversion following the specification order
- Thread safety maintained through outer mutex locks
The helper methods
isEntryTombstonedandconvertEntryToLiveMapValueprovide good separation of concerns and avoid code duplication.
For debugging tests.
The log output was really hard to read; asked Cursor to do this with the following guidance: - one InboundObjectMessage per line - I don't need to see any of the type names in the output, just their properties - I don't need to see "Optional" in the output; if it's nil just write nil, if it's non-nil then just display the wrapped value Have not checked its output in any detail.
The bumped ably-cocoa submodule requires this.
This gives us atomicity instead of breaking it up into multiple mutex acquisitions. Should have done this when I implemented these methods.
Make sure that we perform the subscription synchronously so as not to miss events. This is a nuance that I missed when looking at Cursor's generated code in fa255c1. The way in which I've chosen to do this is not particularly elegant (and to be honest Swift doesn't make it very easy to do so) but I think that it's the closest to the JS code, which hopefully will mean Cursor will be able to easily apply this pattern when translating further JS tests.
The features that these depended on have now been implemented.
lawrence-forooghian
force-pushed
the
ECO-5465-enable-disabled-integration-tests
branch
from
e2a78f7 to
c4bab5f
Compare
lawrence-forooghian
deleted the
ECO-5465-enable-disabled-integration-tests
branch
Note: This PR is based on top of #51; please review that one first.
This enables the integration tests that I'd ported from JS in fa255c1 but which were disabled because the features they depended on hadn't been implemented yet. The enabling of the tests is preceded by some improvements to logging (which I used to help me debug some failing tests), and some fixes for test failures. See commit messages for more details.
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Tests