Concept/Prototype for testing utilities #870
Merged
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.
This is basically the concept, in my mind, of what our testing utilities should look like...
To summarize...
In the case of DD today, the
RecordingObserver
classes are largely redundant, compared to theAggregator
classes (except forValueRecordingObserver
) so they could perhaps be removed, and kept as a concept for DD2. Otherwise, the intent here would be that we'd add moreRecordingObserver
classes andValidateChangeSets()
methods, as needed, for the additional types of changesets, such asIGroupedChangeSet<TObject, TKey>
.I think the advantage of the
RecordingObserver
implementation, over theAggregator
implementation (aside from consistency) is having them implemented without leveragingObservable.Create()
orObserver.Create()
. Using those, we potentially risk faults in operators under test being hidden by the safeguards that are baked intoObservable.Create()
andObserver.Create()
. @dwcullop argues that since DD uses these exclusively, there's no point in testing for behaviors that they prevent, and he's turned around my opinion to agree. However, I still think it's worth having these implementations be safeguard-free, given that the code to do so remains very simple, and that there could be bad-behavior scenarios we're just not thinking of.The
ValidateChangeSets()
methods are definitely a good fit for existing DD, since they can be added to existing tests, without having to touch how theAggregator
classes are implemented, or consumed.I plan to take a pass over this code again, depending on the feedback I get, to try and de-duplicate it. I'd like, for example, the logic for "apply a change to an
IDictionary<>
or anIList<T>
" to be shared code somewhere in DD internals, that we can reuse.