Skip to content
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

Concept/Prototype for testing utilities #870

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

JakenVeina
Copy link
Collaborator

This is basically the concept, in my mind, of what our testing utilities should look like...

To summarize...

public static class ObservableExtensions
{
    public static IDisposable RecordCacheItems<TObject, TKey>(
        this    IObservable<IChangeSet<TObject, TKey>>      source,
        out     CacheItemRecordingObserver<TObject, TKey>   observer,
                IScheduler?                                 scheduler = null);

    public static IDisposable RecordListItems<TObject, TKey>(
        this    IObservable<IChangeSet<TObject, TKey>>      source,
        out     ListItemRecordingObserver<TObject, TKey>    observer,
                IScheduler?                                 scheduler = null);

    public static IDisposable RecordValues<T>(
        this    IObservable<T>              source,
        out     ValueRecordingObserver<T>   observer,
                IScheduler?                 scheduler = null);

    public static IObservable<IChangeSet<T>> ValidateChangeSets<T>(this IObservable<IChangeSet<T>> source);

    public static IObservable<IChangeSet<TObject, TKey>> ValidateChangeSets<TObject, TKey>(
        this    IObservable<IChangeSet<TObject, TKey>>  source,
                Func<TObject, TKey>                     keySelector);

    public static IObservable<T> ValidateSynchronization<T>(this IObservable<T> source);
}

public abstract class RecordingObserverBase<T>
    : IObserver<T>
{
    public Exception? Error { get; }
    public bool HasCompleted { get; }
    public bool HasFinalized { get; }
    public IReadOnlyList<Recorded<Notification<T>>> Notifications { get; }
}

public sealed class ValueRecordingObserver<T>
    : RecordingObserverBase<T>
{
    public IReadOnlyList<T> RecordedValues { get; }
}

public sealed class CacheItemRecordingObserver<TObject, TKey>
        : RecordingObserverBase<IChangeSet<TObject, TKey>>
    where TObject : notnull
    where TKey : notnull
{
    public IReadOnlyList<IChangeSet<TObject, TKey>> RecordedChangeSets { get; }
    public IReadOnlyDictionary<TKey, TObject> RecordedItemsByKey { get; }
    public IReadOnlyList<TObject> RecordedItemsSorted { get; }
}

public sealed class ListItemRecordingObserver<T>
        : RecordingObserverBase<IChangeSet<T>>
    where T : notnull
{
    public IReadOnlyList<IChangeSet<T>> RecordedChangeSets { get; }
    public IReadOnlyList<T> RecordedItems { get; }
}

In the case of DD today, the RecordingObserver classes are largely redundant, compared to the Aggregator classes (except for ValueRecordingObserver so they could perhaps be removed, and kept as a concept for DD2. Otherwise, the intent here would be that we'd add more RecordingObserver classes and ValidateChangeSets() methods, as needed, for the additional types of changesets, such as IGroupedChangeSet<TObject, TKey>.

I think the advantage of the RecordingObserver implementation, over the Aggregator implementation (aside from consistency) is having them implemented without leveraging Observable.Create() or Observer.Create(). Using those, we potentially risk faults in operators under test being hidden by the safeguards that are baked into Observable.Create() and Observer.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 the Aggregator 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 an IList<T> to be shared code somewhere in DD internals, that we can reuse.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant