From 30be4112467582434d24d8a60592bb5a4b9480d3 Mon Sep 17 00:00:00 2001 From: Jaime <52668514+jsgalarraga@users.noreply.github.com> Date: Wed, 29 May 2024 12:45:32 +0200 Subject: [PATCH] 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 --------- Co-authored-by: Alejandro Santiago --- lib/crossword/view/crossword_page.dart | 1 + .../bloc/random_word_selection_bloc.dart | 8 ++- .../lib/src/crossword_repository.dart | 40 ++++++-------- .../test/src/crossword_repository_test.dart | 55 +++++++++---------- test/helpers/pump_app.dart | 6 +- .../bloc/random_word_selection_bloc_test.dart | 28 +++++++--- 6 files changed, 75 insertions(+), 63 deletions(-) diff --git a/lib/crossword/view/crossword_page.dart b/lib/crossword/view/crossword_page.dart index 74fcbc1a7..de5a0aa71 100644 --- 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(), + boardInfoRepository: context.read(), ), ), 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 index f3b48f8bf..21d7b4d4a 100644 --- 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 { RandomWordSelectionBloc({ required CrosswordRepository crosswordRepository, + required BoardInfoRepository boardInfoRepository, }) : _crosswordRepository = crosswordRepository, + _boardInfoRepository = boardInfoRepository, super(const RandomWordSelectionState()) { on(_onRandomWordRequested); } final CrosswordRepository _crosswordRepository; + final BoardInfoRepository _boardInfoRepository; Future _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 index 5e5a3c215..abc09bc92 100644 --- 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 getRandomUncompletedSection() async { + Future getRandomUncompletedSection( + Point 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,17 +60,15 @@ 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, @@ -84,17 +76,19 @@ class CrosswordRepository { }); }); - 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 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 index d9a3017ba..84ff6a7cb 100644 --- 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,14 +54,14 @@ void main() { }); group('getRandomEmptySection', () { - const boardHalfSize = 3; + const bottomRight = Point(2, 2); Future 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( @@ -70,6 +69,8 @@ void main() { 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 index addc72eba..7d008146a 100644 --- 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 index f6e93a260..e77a443d1 100644 --- 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: () => [ @@ -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: () => [