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

Equality for objects with a list parameter #46

Closed
HossamElharmeil opened this issue Dec 24, 2019 · 14 comments
Closed

Equality for objects with a list parameter #46

HossamElharmeil opened this issue Dec 24, 2019 · 14 comments
Assignees
Labels
question Further information is requested
Projects

Comments

@HossamElharmeil
Copy link

I'm playing around with the new library bloc_test for flutter and I implemented the following test

blocTest('should return ReservationsLoadSucess when the use case returns a list of reservationsList',

    build: () {
      when(mockGetReservations(any)).thenAnswer((_) async => Right(reservationsList));
      return ReservationBloc(getReservations: mockGetReservations);
    },
    act: (bloc) async {
      bloc.add(ReservationsRequested(user));
    },
    expect: [
      ReservationsInitial(),
      ReservationsLoadInProgress(),
      ReservationsLoadSuccess(reservationsList),
    ],
  );

This is the implementation of ReservationsLoadSuccess

class ReservationsLoadSuccess extends ReservationState {
  final List<Reservation> list;

  ReservationsLoadSuccess(this.list);

  @override
  List<Object> get props => [list];
}

Where ReservationState extends Equatable Now, when running the test, you get the following error

should return ReservationsLoadSucess when the use case returns a list of reservationsList:

ERROR: Expected: [
            ReservationsInitial:ReservationsInitial,
            ReservationsLoadInProgress:ReservationsLoadInProgress,
            ReservationsLoadSuccess:ReservationsLoadSuccess
          ]
  Actual: [
            ReservationsInitial:ReservationsInitial,
            ReservationsLoadInProgress:ReservationsLoadInProgress,
            ReservationsLoadSuccess:ReservationsLoadSuccess
          ]
   Which: was ReservationsLoadSuccess:<ReservationsLoadSuccess> instead of ReservationsLoadSuccess:<ReservationsLoadSuccess> at location [2]

Basically saying that the state ReservationsLoadSuccess at position 2 in the actual list is not equal to its peer in the expected list.

I tried overriding the == operator in the ReservationsLoadSuccess class as follows

class ReservationsLoadSuccess extends ReservationState {
  final List<Reservation> list;

  ReservationsLoadSuccess(this.list);

  final Function eq = const ListEquality().equals;
  @override
  List<Object> get props => [];

  @override
  bool operator ==(Object other) =>
      identical(this, other) ||
      other is ReservationsLoadSuccess &&
          runtimeType == other.runtimeType &&
          eq(list, other.list);
}

But that didn't seem to work and running the test still outputted the same error. The only way I got it to work is to leave the props method returning an empty list or adding any other dummy variable and pass it to the props list.

Is there any way I can make the class equatable in regards to the list parameter?

@felangel
Copy link
Owner

Hi @HossamElharmeil 👋
Thanks for opening an issue!

I believe the issue is your Reservation class is not extending Equatable. Can you please confirm? Thanks!

@felangel felangel self-assigned this Dec 25, 2019
@felangel felangel added the question Further information is requested label Dec 25, 2019
@HossamElharmeil
Copy link
Author

I thought that might be the case too but it does extend equatable when I double checked :D
Here is the implementation

class Reservation extends Equatable {
  final String salon;
  final String userID;
  final DateTime dateTime;

  Reservation({this.salon, this.userID, this.dateTime});

  @override
  String toString() {
    return "Reservation => userID: $userID, salon: $salon, date: $dateTime";
  }

  Map<String, dynamic> toJson() {
    return {
      'userID': userID,
      'salon': salon,
      'dateTime': dateTime,
    };
  }

  @override
  List<Object> get props => [salon, userID, dateTime];
}

@felangel
Copy link
Owner

@HossamElharmeil what about DateTime? I think that might be the problem.

@HossamElharmeil
Copy link
Author

HossamElharmeil commented Dec 25, 2019

I tried changing the props list only to contain the userID parameter but that did not work either which is really interesting. I think the issue is with the reservation class as you suggested but I'll have to figure out how

class Reservation extends Equatable {
  final String salon;
  final String userID;
  final DateTime dateTime;

  Reservation({this.salon, this.userID, this.dateTime});

  @override
  String toString() {
    return "Reservation => userID: $userID, salon: $salon, date: $dateTime";
  }

  Map<String, dynamic> toJson() {
    return {
      'userID': userID,
      'salon': salon,
      'dateTime': dateTime,
    };
  }

  @override
  List<Object> get props => [userID];
}

@HossamElharmeil
Copy link
Author

So, I implemented the exact same class without the DateTime parameter and this time it actually worked! I guess having a parameter that is not equatable by value in the class means the class itself cannot be equatable by value regardless of the parameters passed to props?

Anyway, thank you so much for your help, Felix. At least I know how it works now xD

class ReservationTest extends Equatable {
  final String salon;
  final String userID;

  ReservationTest({this.salon, this.userID});

  Map<String, dynamic> toJson() {
    return {
      'userID': userID,
      'salon': salon,
    };
  }

  @override
  String toString() {
    return "Reservation => userID: $userID, salon: $salon";
  }

  @override
  List<Object> get props => [salon, userID];
}

@felangel
Copy link
Owner

@HossamElharmeil yup you can create your own EquatableDateTime by using the EquatableMixin. Closing for now but feel free to comment with additional questions/comments and I'm happy to continue the conversation 👍

@felangel felangel added this to Done in equatable Dec 25, 2019
@HossamElharmeil
Copy link
Author

HossamElharmeil commented Dec 26, 2019

Somehow I got it to work by instantiating the list directly inside the test instead of declaring it in the main() function of the test and then passing it as a parameter.

blocTest('should return ReservationsLoadSucess when the use case returns a list of reservationsList',

    build: () {
      when(mockGetReservations(any)).thenAnswer((_) async => Right([Reservation(user: "12345", salon: "salon", dateTime: DateTime(1998, 8,8))]));
      return ReservationBloc(getReservations: mockGetReservations);
    },
    act: (bloc) async {
      bloc.add(ReservationsRequested(user));
    },
    expect: [
      ReservationsInitial(),
      ReservationsLoadInProgress(),
      ReservationsLoadSuccess([Reservation(user: "12345", salon: "salon", dateTime: DateTime(1998, 8,8))]),
    ],
  );

Turns out that the implementation of DateTime does actually override the equals operator for value equality, it had to do with the declaration of the list which is kinda weird. Maybe I'll have to do some research about runtime and compile time stuff in dart when it comes to initializing something in a list.

@rodrigobastosv
Copy link

Hi @felangel can we reopen this? Turns out i'm having the exact same problem as @HossamElharmeil.
When i instantiate the list directly on the test it works, if i initialize the list inside setUp it doesn't work.

It may not be a problem with Equatable, but this behavior it's quite strange

@felangel
Copy link
Owner

@rodrigobastosv can you provide a link to a sample which reproduces the issue?

@rodrigobastosv
Copy link

Sure, just let me get back at home and i'll provided it. Thanks

@rodrigobastosv
Copy link

Hi @felangel , this is my repo https://github.com/rodrigobastosv/firebase_bloc

I created branchs with the problematic tests only

  • The branch passing-tests has the tests working with some workarounds
  • The branch failling-tests has the tests as i expected to be, but that are failing to pass

See that my model uses equatable with only id, so i expected to just work.

There's another test that is oddly failing if you could please help telling me why.

Thanks very much man!

@felangel
Copy link
Owner

@rodrigobastosv will take a look shortly, thanks!

@felangel
Copy link
Owner

@rodrigobastosv took a look and the problem is due to the fact that you're using an immutable list for congressPeople.

import 'package:bloc_test/bloc_test.dart';
import 'package:firebase_bloc/model/congressman_model.dart';
import 'package:firebase_bloc/ui/bloc/bloc.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:mockito/mockito.dart';

import '../mocks.dart';

void main() {
  group('CongressBloc tests', () {
    MockCongressRepository mockCongressRepository;
    final List<CongressmanModel> congressPeople = [CongressmanModel(id: 1, nome: 'Test')];

    setUp(() {
      mockCongressRepository = MockCongressRepository();
    });

    blocTest(
      'CongressFetch event results in [InitialCongressState, CongressLoadingState, CongressLoadedState]',
      build: () {
        when(mockCongressRepository.getCongressPeople())
            .thenAnswer((_) => Future.value(congressPeople));
        return CongressBloc((mockCongressRepository));
      },
      act: (bloc) {
        bloc.add(CongressFetch());
        return;
      },
      expect: [
        InitialCongressState(),
        CongressLoadingState(),
        CongressLoadedState(congressPeople)
      ],
    );

    blocTest(
      'CongressFetch event results in [InitialCongressState, CongressLoadingState, CongressLoadedFailledState] when error happens',
      build: () {
        when(mockCongressRepository.getCongressPeople())
            .thenThrow(Exception('Opsie Dasie'));
        return CongressBloc((mockCongressRepository));
      },
      act: (bloc) {
        bloc.add(CongressFetch());
        return;
      },
      expect: [
        InitialCongressState(),
        CongressLoadingState(),
        CongressLoadedFailledState('Exception: Opsie Dasie')
      ],
    );
  });
}

Also (tests still pass without this) but I noticed that you're not passing all props in CongressmanModel

@override
List<Object> get props => [id];

Hope that helps 👍

@rodrigobastosv
Copy link

Thanks for the reply @felangel !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
equatable
  
Done
Development

No branches or pull requests

3 participants