fix: find new word finds different word each time (#533)

* fix: find new word finds different word each time * test: make test about getting a random section more explicit * docs: update Co-authored-by: Alejandro Santiago <dev@alestiago.com> --------- Co-authored-by: Alejandro Santiago <dev@alestiago.com>
VGVentures · May 29, 2024 · 30be411 · 30be411
1 parent 94a3c97
commit 30be411
Show file tree

Hide file tree

Showing 6 changed files with 75 additions and 63 deletions.
diff --git a/lib/crossword/view/crossword_page.dart b/lib/crossword/view/crossword_page.dart
@@ -49,6 +49,7 @@ class CrosswordPage extends StatelessWidget {
           BlocProvider(
             create: (_) => RandomWordSelectionBloc(
               crosswordRepository: context.read<CrosswordRepository>(),
+              boardInfoRepository: context.read<BoardInfoRepository>(),
             ),
           ),
           BlocProvider(

diff --git a/lib/random_word_selection/bloc/random_word_selection_bloc.dart b/lib/random_word_selection/bloc/random_word_selection_bloc.dart
@@ -1,6 +1,7 @@
 import 'dart:async';
 
 import 'package:bloc/bloc.dart';
+import 'package:board_info_repository/board_info_repository.dart';
 import 'package:crossword_repository/crossword_repository.dart';
 import 'package:equatable/equatable.dart';
 import 'package:game_domain/game_domain.dart';
@@ -12,12 +13,15 @@ class RandomWordSelectionBloc
     extends Bloc<RandomWordSelectionEvent, RandomWordSelectionState> {
   RandomWordSelectionBloc({
     required CrosswordRepository crosswordRepository,
+    required BoardInfoRepository boardInfoRepository,
   })  : _crosswordRepository = crosswordRepository,
+        _boardInfoRepository = boardInfoRepository,
         super(const RandomWordSelectionState()) {
     on<RandomWordRequested>(_onRandomWordRequested);
   }
 
   final CrosswordRepository _crosswordRepository;
+  final BoardInfoRepository _boardInfoRepository;
 
   Future<void> _onRandomWordRequested(
     RandomWordRequested event,
@@ -26,8 +30,10 @@ class RandomWordSelectionBloc
     try {
       emit(state.copyWith(status: RandomWordSelectionStatus.loading));
 
+      final bottomRight = await _boardInfoRepository.getBottomRight();
       final randomSection =
-          await _crosswordRepository.getRandomUncompletedSection();
+          await _crosswordRepository.getRandomUncompletedSection(bottomRight);
+
       if (randomSection != null) {
         emit(
           state.copyWith(

diff --git a/packages/crossword_repository/lib/src/crossword_repository.dart b/packages/crossword_repository/lib/src/crossword_repository.dart
@@ -24,34 +24,28 @@ class CrosswordRepository {
 
   final Random _rng;
 
-  // coverage:ignore-start
-  // Ignore coverage due to unhandled case in the fake_cloud_firestore package
-  // Expected tests are still created and commented in corresponding test file
   /// Returns the position of a random section that has empty words.
-  Future<BoardSection?> getRandomUncompletedSection() async {
+  Future<BoardSection?> getRandomUncompletedSection(
+    Point<int> bottomRight,
+  ) async {
     final docsCount = await sectionCollection.count().get();
     final sectionCount = docsCount.count;
     if (sectionCount == null) {
       return null;
     }
 
-    // Calculate the length of the board knowing that it's a square
-    final boardSize = sqrt(sectionCount).floor();
-
-    final maxPos = (boardSize / 2).floor();
-    final minPos = 0 - maxPos;
+    const minPos = Point(0, 0);
+    final maxPos = bottomRight;
 
     final positions = <(int, int)>[];
 
     // Get all the possible positions from top left to bottom right
-    for (var x = minPos; x < maxPos; x++) {
-      for (var y = minPos; y < maxPos; y++) {
+    for (var x = minPos.x; x <= maxPos.x; x++) {
+      for (var y = minPos.y; y <= maxPos.y; y++) {
         positions.add((x, y));
       }
     }
 
-    positions.shuffle(_rng);
-
     const batchSize = 20;
 
     // Get batches of 20 random sections until finding one with unsolved word
@@ -66,35 +60,35 @@ class CrosswordRepository {
       final result = await sectionCollection
           .where(
             'position',
-            whereIn: batchPositions
-                .map(
-                  (e) => {
-                    'x': e.$1,
-                    'y': e.$2,
-                  },
-                )
-                .toList(),
+            whereIn: batchPositions.map((e) => {'x': e.$1, 'y': e.$2}),
           )
           .get();
 
+      // The coverage is ignored due to a bug in the fake_cloud_firestore
+      // package. It does not handle the case when the whereIn query
+      // has to match a map, which makes it difficult to test query results.
+      // https://github.com/atn832/fake_cloud_firestore/issues/301.
+      // coverage:ignore-start
       final sections = result.docs.map((sectionDoc) {
         return BoardSection.fromJson({
           'id': sectionDoc.id,
           ...sectionDoc.data(),
         });
       });
 
-      final section = sections.firstWhereOrNull(
+      final randomizedSections = sections.toList()..shuffle(_rng);
+
+      final section = randomizedSections.firstWhereOrNull(
         (section) => section.words.any((word) => word.solvedTimestamp == null),
       );
 
       if (section != null) {
         return section;
       }
+      // coverage:ignore-end
     }
     return null;
   }
-  // coverage:ignore-end
 
   /// Watches a section of the crossword board
   Stream<BoardSection> watchSection(String id) {

diff --git a/packages/crossword_repository/test/src/crossword_repository_test.dart b/packages/crossword_repository/test/src/crossword_repository_test.dart
@@ -1,7 +1,6 @@
 // ignore_for_file: prefer_const_constructors
 import 'dart:math';
 
-import 'package:cloud_firestore/cloud_firestore.dart';
 import 'package:crossword_repository/crossword_repository.dart';
 import 'package:fake_cloud_firestore/fake_cloud_firestore.dart';
 import 'package:flutter_test/flutter_test.dart';
@@ -30,7 +29,7 @@ void main() {
     );
     const sectionsCollection = 'boardChunks';
 
-    late FirebaseFirestore firebaseFirestore;
+    late FakeFirebaseFirestore firebaseFirestore;
     late CrosswordRepository crosswordRepository;
     late Random rng;
 
@@ -55,21 +54,23 @@ void main() {
     });
 
     group('getRandomEmptySection', () {
-      const boardHalfSize = 3;
+      const bottomRight = Point(2, 2);
 
       Future<void> setUpSections({int solveUntil = 0}) async {
         var solvedIndex = 0;
-        for (var x = -boardHalfSize; x < boardHalfSize; x++) {
-          for (var y = -boardHalfSize; y < boardHalfSize; y++) {
-            final section = boardSection.copyWith(
-              id: '${boardSection.id}-$x-$y',
+        for (var x = 0; x <= bottomRight.x; x++) {
+          for (var y = 0; y <= bottomRight.y; y++) {
+            final section = BoardSection(
+              id: '$x-$y',
               position: Point(x, y),
               words: [
                 word.copyWith(
                   position: Point(x, y),
                   solvedTimestamp: solvedIndex <= solveUntil ? 1 : null,
                 ),
               ],
+              size: 10,
+              borderWords: const [],
             );
 
             await firebaseFirestore
@@ -81,39 +82,33 @@ void main() {
         }
       }
 
-      /*
-
-      Tests failing due to unhandled case in fake_cloud_firestore package
-
-      test('returns a random section', () async {
-        await setUpSections(solveUntil: 3);
+      test('returns normally', () async {
+        await setUpSections(solveUntil: 1);
         when(() => rng.nextInt(any())).thenReturn(3);
 
-        final pos = await crosswordRepository.getRandomEmptySection();
-        expect(pos, equals(Point(-3, 1)));
+        await expectLater(
+          () => crosswordRepository.getRandomUncompletedSection(bottomRight),
+          returnsNormally,
+        );
       });
 
-      test('returns last section if every others only have solved words',
-          () async {
-        const totalSections = boardHalfSize * 2 * boardHalfSize * 2;
-        await setUpSections(solveUntil: totalSections - 2);
-        when(() => rng.nextInt(any())).thenReturn(3);
-
-        final pos = await crosswordRepository.getRandomEmptySection();
-        expect(pos, equals(Point(2, 2)));
-      });*/
-
-      test('returns null if every sections only have solved words', () async {
-        const totalSections = boardHalfSize * 2 * boardHalfSize * 2;
+      test('returns null if all sections have been solved', () async {
+        final totalSections = (bottomRight.x + 1) * (bottomRight.y + 1);
         await setUpSections(solveUntil: totalSections);
         when(() => rng.nextInt(any())).thenReturn(3);
 
-        final pos = await crosswordRepository.getRandomUncompletedSection();
+        final pos = await crosswordRepository.getRandomUncompletedSection(
+          bottomRight,
+        );
+
         expect(pos, isNull);
       });
 
-      test('returns null if no section found', () async {
-        final pos = await crosswordRepository.getRandomUncompletedSection();
+      test('returns null if no sections are found', () async {
+        final pos = await crosswordRepository.getRandomUncompletedSection(
+          bottomRight,
+        );
+
         expect(pos, isNull);
       });
     });

diff --git a/test/helpers/pump_app.dart b/test/helpers/pump_app.dart
@@ -75,7 +75,8 @@ extension PumpApp on WidgetTester {
     final mockedCrosswordResource = _MockCrosswordResource();
     final mockedCrosswordRepository = _MockCrosswordRepository();
     registerFallbackValue(Point(0, 0));
-    when(mockedCrosswordRepository.getRandomUncompletedSection).thenAnswer(
+    when(() => mockedCrosswordRepository.getRandomUncompletedSection(any()))
+        .thenAnswer(
       (_) => Future.value(
         BoardSection(
           id: '',
@@ -218,7 +219,8 @@ extension PumpRoute on WidgetTester {
       ),
     );
     final mockedCrosswordRepository = _MockCrosswordRepository();
-    when(mockedCrosswordRepository.getRandomUncompletedSection).thenAnswer(
+    when(() => mockedCrosswordRepository.getRandomUncompletedSection(any()))
+        .thenAnswer(
       (_) => Future.value(
         BoardSection(
           id: '',

diff --git a/test/random_word_selection/bloc/random_word_selection_bloc_test.dart b/test/random_word_selection/bloc/random_word_selection_bloc_test.dart
@@ -1,6 +1,7 @@
 // ignore_for_file: prefer_const_constructors
 
 import 'package:bloc_test/bloc_test.dart';
+import 'package:board_info_repository/board_info_repository.dart';
 import 'package:crossword_repository/crossword_repository.dart';
 import 'package:flutter_test/flutter_test.dart';
 import 'package:game_domain/game_domain.dart';
@@ -9,6 +10,8 @@ import 'package:mocktail/mocktail.dart';
 
 class _MockCrosswordRepository extends Mock implements CrosswordRepository {}
 
+class _MockBoardInfoRepository extends Mock implements BoardInfoRepository {}
+
 class _FakeUnsolvedWord extends Fake implements Word {
   @override
   int? get solvedTimestamp => null;
@@ -17,9 +20,19 @@ class _FakeUnsolvedWord extends Fake implements Word {
 void main() {
   group('$RandomWordSelectionBloc', () {
     late CrosswordRepository crosswordRepository;
+    late BoardInfoRepository boardInfoRepository;
+
+    setUpAll(() {
+      registerFallbackValue(Point(0, 0));
+    });
 
     setUp(() {
       crosswordRepository = _MockCrosswordRepository();
+      boardInfoRepository = _MockBoardInfoRepository();
+
+      when(() => boardInfoRepository.getBottomRight()).thenAnswer(
+        (_) => Future.value(Point(2, 2)),
+      );
     });
 
     group('$RandomWordRequested', () {
@@ -37,11 +50,11 @@ void main() {
         ' getRandomUncompletedSection succeeds',
         build: () => RandomWordSelectionBloc(
           crosswordRepository: crosswordRepository,
+          boardInfoRepository: boardInfoRepository,
         ),
         setUp: () {
-          when(crosswordRepository.getRandomUncompletedSection).thenAnswer(
-            (_) => Future.value(section),
-          );
+          when(() => crosswordRepository.getRandomUncompletedSection(any()))
+              .thenAnswer((_) => Future.value(section));
         },
         act: (bloc) => bloc.add(RandomWordRequested()),
         expect: () => <RandomWordSelectionState>[
@@ -58,9 +71,10 @@ void main() {
         ' getRandomUncompletedSection fails',
         build: () => RandomWordSelectionBloc(
           crosswordRepository: crosswordRepository,
+          boardInfoRepository: boardInfoRepository,
         ),
         setUp: () {
-          when(crosswordRepository.getRandomUncompletedSection)
+          when(() => crosswordRepository.getRandomUncompletedSection(any()))
               .thenThrow(Exception());
         },
         act: (bloc) => bloc.add(RandomWordRequested()),
@@ -75,11 +89,11 @@ void main() {
         ' getRandomUncompletedSection returns null',
         build: () => RandomWordSelectionBloc(
           crosswordRepository: crosswordRepository,
+          boardInfoRepository: boardInfoRepository,
         ),
         setUp: () {
-          when(crosswordRepository.getRandomUncompletedSection).thenAnswer(
-            (_) => Future.value(),
-          );
+          when(() => crosswordRepository.getRandomUncompletedSection(any()))
+              .thenAnswer((_) => Future.value());
         },
         act: (bloc) => bloc.add(RandomWordRequested()),
         expect: () => <RandomWordSelectionState>[