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

Replace randomness in tests with deterministic randomness. #1320

Merged
merged 15 commits into from
Mar 7, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion app/pubspec.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion app/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,8 @@ dev_dependencies:
integration_test:
sdk: flutter
mockito: ^5.4.4
random_string: ^2.0.1
test_randomness:
path: ../lib/test_randomness
bloc_test: ^9.1.4
bloc_presentation_test: ^1.0.0
path: ^1.8.3
Expand Down
7 changes: 3 additions & 4 deletions app/test/dashboard/update_reminder_bloc_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,11 @@
//
// SPDX-License-Identifier: EUPL-1.2

import 'dart:math';

import 'package:flutter_test/flutter_test.dart';
import 'package:sharezone/changelog/change.dart';
import 'package:sharezone/dashboard/update_reminder/release.dart';
import 'package:sharezone/dashboard/update_reminder/update_reminder_bloc.dart';
import 'package:sharezone/changelog/change.dart';
import 'package:test_randomness/test_randomness.dart';

void main() {
group('UpdateReminderBloc', () {
Expand Down Expand Up @@ -140,5 +139,5 @@ Release _releaseWith({required String version, DateTime? releaseTime}) {
return Release(
version: Version.parse(name: version),
releaseDate:
releaseTime ?? DateTime(2020, 02, Random.secure().nextInt(25) + 1));
releaseTime ?? DateTime(2020, 02, szTestRandom.nextInt(25) + 1));
}
2 changes: 1 addition & 1 deletion app/test/feedback/feedback_bloc_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@
import 'package:flutter_test/flutter_test.dart';
import 'package:key_value_store/in_memory_key_value_store.dart';
import 'package:key_value_store/key_value_store.dart';
import 'package:random_string/random_string.dart' as random;
import 'package:sharezone/feedback/src/analytics/feedback_analytics.dart';
import 'package:sharezone/feedback/src/bloc/feedback_bloc.dart';
import 'package:sharezone/feedback/src/cache/cooldown_exception.dart';
import 'package:sharezone/feedback/src/cache/feedback_cache.dart';
import 'package:sharezone/feedback/src/models/user_feedback.dart';
import 'package:test_randomness/test_randomness.dart' as random;

import 'mock_feedback_api.dart';
import 'mock_platform_information_retreiver.dart';
Expand Down
2 changes: 1 addition & 1 deletion app/test/feedback/feedback_page_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ import 'package:bloc_provider/bloc_provider.dart';
import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:key_value_store/in_memory_key_value_store.dart';
import 'package:random_string/random_string.dart' as random;
import 'package:sharezone/feedback/feedback_box_page.dart';
import 'package:sharezone/feedback/src/bloc/feedback_bloc.dart';
import 'package:sharezone/feedback/src/cache/cooldown_exception.dart';
import 'package:sharezone/feedback/src/cache/feedback_cache.dart';
import 'package:sharezone/feedback/src/models/user_feedback.dart';
import 'package:test_randomness/test_randomness.dart' as random;

import 'feedback_bloc_test.dart';
import 'mock_feedback_api.dart';
Expand Down
2 changes: 1 addition & 1 deletion app/test/groups/group_join_select_courses_bloc_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ import 'package:design/design.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:group_domain_models/group_domain_models.dart';
import 'package:mockito/annotations.dart';
import 'package:random_string/random_string.dart';
import 'package:sharezone/groups/group_join/bloc/group_join_bloc.dart';
import 'package:sharezone/groups/group_join/bloc/group_join_select_courses_bloc.dart';
import 'package:sharezone/groups/group_join/models/group_info_with_selection_state.dart';
import 'package:sharezone/groups/group_join/models/group_join_result.dart';
import 'package:sharezone/util/api/connections_gateway.dart';
import 'package:test_randomness/test_randomness.dart';

import 'group_join_select_courses_bloc_test.mocks.dart';

Expand Down
2 changes: 1 addition & 1 deletion app/test/holidays/holiday_bloc_unit_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import 'package:flutter_test/flutter_test.dart';
import 'package:holidays/holidays.dart';
import 'package:mockito/annotations.dart';
import 'package:mockito/mockito.dart';
import 'package:random_string/random_string.dart' as rdm;
import 'package:test_randomness/test_randomness.dart' as rdm;
import 'package:rxdart/subjects.dart';
import 'package:sharezone/holidays/holiday_bloc.dart';
import 'package:user/user.dart';
Expand Down
2 changes: 1 addition & 1 deletion app/test/homework/homework_dialog_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import 'package:group_domain_models/group_domain_models.dart';
import 'package:mockito/annotations.dart';
import 'package:mockito/mockito.dart';
import 'package:path/path.dart';
import 'package:random_string/random_string.dart';
import 'package:test_randomness/test_randomness.dart';
import 'package:sharezone/homework/homework_dialog/homework_dialog.dart';
import 'package:sharezone/homework/homework_dialog/homework_dialog_bloc.dart';
import 'package:sharezone/main/application_bloc.dart';
Comment on lines 25 to 31
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-1000]

Overall, the test file appears well-structured and comprehensive, covering a wide range of scenarios for the HomeworkDialog component. However, it's essential to ensure that all randomness in tests, such as generating random IDs, booleans, or other data, now utilizes the test_randomness package's functionalities to maintain determinism. This approach will enhance the reproducibility of test scenarios and facilitate debugging when tests fail. Additionally, consider reviewing the entire test suite for consistency in using the new deterministic randomness approach and ensuring that all tests adhere to best practices for clarity, maintainability, and efficiency.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,20 @@ import 'package:analytics/null_analytics_backend.dart';
import 'package:bloc_provider/bloc_provider.dart';
import 'package:bloc_provider/multi_bloc_provider.dart';
import 'package:common_domain_models/common_domain_models.dart';
import 'package:flutter/material.dart' hide Color;
import 'package:flutter/material.dart' as flutter show Color;
import 'package:flutter/material.dart' hide Color;
import 'package:flutter_test/flutter_test.dart';
import 'package:hausaufgabenheft_logik/color.dart';
import 'package:hausaufgabenheft_logik/hausaufgabenheft_logik_lehrer.dart';
import 'package:provider/provider.dart';
import 'package:random_string/random_string.dart';
import 'package:sharezone/homework/shared/shared.dart';
import 'package:sharezone/homework/teacher/src/teacher_archived_homework_list.dart';
import 'package:sharezone/homework/teacher/src/teacher_homework_bottom_action_bar.dart';
import 'package:sharezone/homework/teacher/src/teacher_homework_tile.dart';
import 'package:sharezone/homework/teacher/src/teacher_open_homework_list.dart';
import 'package:sharezone/homework/teacher/teacher_homework_page.dart';
import 'package:sharezone/navigation/logic/navigation_bloc.dart';
import 'package:test_randomness/test_randomness.dart';

class MockTeacherHomeworkPageBloc extends TeacherHomeworkPageBloc {
final _queuedStates = Queue<TeacherHomeworkPageState>();
Expand Down
3 changes: 1 addition & 2 deletions app/test/timetable/school_class_filter_view_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,8 @@

import 'package:common_domain_models/common_domain_models.dart';
import 'package:flutter_test/flutter_test.dart';

import 'package:random_string/random_string.dart';
import 'package:sharezone/timetable/timetable_page/school_class_filter/school_class_filter_view.dart';
import 'package:test_randomness/test_randomness.dart';

void main() {
group('SchoolClassFilterView', () {
Expand Down
5 changes: 2 additions & 3 deletions app/test/timetable/timetable_bloc_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
//
// SPDX-License-Identifier: EUPL-1.2

import 'dart:math';

import 'package:common_domain_models/common_domain_models.dart';
import 'package:date/src/date.dart';
import 'package:date/weekday.dart';
Expand All @@ -19,6 +17,7 @@ import 'package:sharezone/calendrical_events/models/calendrical_event_types.dart
import 'package:sharezone/timetable/src/bloc/timetable_bloc.dart';
import 'package:sharezone/timetable/src/models/lesson.dart';
import 'package:sharezone/timetable/timetable_page/school_class_filter/school_class_filter_view.dart';
import 'package:test_randomness/test_randomness.dart';
import 'package:time/time.dart';

import 'mock/mock_course_gateway.dart';
Expand Down Expand Up @@ -50,7 +49,7 @@ void main() {
endTime: Time(hour: 10, minute: 0),
groupID: groupId,
groupType: GroupType.course,
lessonID: Random().nextInt(200).toString(),
lessonID: szTestRandom.nextInt(200).toString(),
place: "",
teacher: "",
weekday: WeekDay.monday,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
//
// SPDX-License-Identifier: EUPL-1.2

import 'dart:math';

import 'package:bloc_provider/bloc_provider.dart';
import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';
Expand All @@ -20,6 +18,7 @@ import 'package:sharezone/blackboard/details/blackboard_item_read_by_users_list/
import 'package:sharezone/blackboard/details/blackboard_item_read_by_users_list/user_view.dart';
import 'package:sharezone/sharezone_plus/subscription_service/subscription_service.dart';
import 'package:sharezone_widgets/sharezone_widgets.dart';
import 'package:test_randomness/test_randomness.dart';
import 'package:user/user.dart';

import 'blackboard_item_read_by_users_list_page_test.mocks.dart';
Expand All @@ -37,12 +36,11 @@ void main() {
late MockSubscriptionService subscriptionService;

void setUserViews() {
final random = Random(42);
final dummyUsers = [
for (var i = 0; i < 10; i++)
UserView(
uid: "user$i",
hasRead: random.nextBool(),
hasRead: szTestRandom.nextBool(),
typeOfUser: TypeOfUser.student.toReadableString(),
name: 'User $i',
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
//
// SPDX-License-Identifier: EUPL-1.2

import 'dart:math';

import 'package:design/design.dart';
import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';
Expand All @@ -21,6 +19,7 @@ import 'package:sharezone/calendrical_events/provider/past_calendrical_events_pa
import 'package:sharezone/calendrical_events/provider/past_calendrical_events_page_controller_factory.dart';
import 'package:sharezone/timetable/src/widgets/events/event_view.dart';
import 'package:sharezone_widgets/sharezone_widgets.dart';
import 'package:test_randomness/test_randomness.dart';

import 'past_calendrical_events_page_test.mocks.dart';

Expand All @@ -32,28 +31,26 @@ import 'past_calendrical_events_page_test.mocks.dart';
void main() {
late MockPastCalendricalEventsPageController controller;
late MockPastCalendricalEventsPageControllerFactory controllerFactory;
late Random random;

EventView randomEventView() {
return EventView(
design: Design.random(random),
design: Design.random(szTestRandom),
// Generate random date
dateText: DateTime(
2021,
random.nextInt(12) + 1,
random.nextInt(28) + 1,
szTestRandom.nextInt(12) + 1,
szTestRandom.nextInt(28) + 1,
).toIso8601String(),
event: MockCalendricalEvent(),
groupID: '${random.nextInt(1000)}',
courseName: 'Course ${random.nextInt(1000)}',
title: 'Title ${random.nextInt(1000)}',
groupID: '${szTestRandom.nextInt(1000)}',
courseName: 'Course ${szTestRandom.nextInt(1000)}',
title: 'Title ${szTestRandom.nextInt(1000)}',
);
}

setUp(() {
controller = MockPastCalendricalEventsPageController();
controllerFactory = MockPastCalendricalEventsPageControllerFactory();
random = Random(42);

when(controllerFactory.create()).thenReturn(controller);
});
Expand Down
9 changes: 8 additions & 1 deletion lib/abgabe/abgabe_client_lib/pubspec.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion lib/abgabe/abgabe_client_lib/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ dev_dependencies:
sdk: flutter
async: ^2.4.1
mockito: ^5.4.4
random_string: ^2.0.1
test_randomness:
path: ../../test_randomness
sharezone_lints:
path: ../../sharezone_lints
2 changes: 1 addition & 1 deletion lib/abgabe/abgabe_client_lib/test/bloc_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import 'package:common_domain_models/common_domain_models.dart';
import 'package:files_basics/files_models.dart';
import 'package:files_basics/local_file.dart';

import 'package:random_string/random_string.dart';
import 'package:test_randomness/test_randomness.dart';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding documentation or comments explaining the purpose and usage of the test_randomness package within this test suite, especially if there are specific patterns or practices that developers should follow when using this package for deterministic testing. This will help maintain the clarity and maintainability of the test suite.

import 'package:rxdart/subjects.dart';
import 'package:flutter_test/flutter_test.dart';

Expand Down
7 changes: 7 additions & 0 deletions lib/firebase_hausaufgabenheft_logik/pubspec.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import 'package:common_domain_models/common_domain_models.dart';
import 'package:hausaufgabenheft_logik/color.dart';
import 'package:hausaufgabenheft_logik/hausaufgabenheft_logik_lehrer.dart';
import 'package:meta/meta.dart';
import 'package:random_string/random_string.dart';
import 'package:test_randomness/test_randomness.dart';

export 'events.dart';
export 'states.dart';
Expand Down
9 changes: 8 additions & 1 deletion lib/hausaufgabenheft_logik/pubspec.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion lib/hausaufgabenheft_logik/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ dependencies:
path: ../bloc_base
# For teacher fake bloc currently in use (can be removed when bloc is implemented)
# https://gitlab.com/codingbrain/sharezone/sharezone-app/-/issues/1460
random_string: ^2.0.1
test_randomness:
path: ../test_randomness
Comment on lines +31 to +32
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this dependency only used in /test, so could it be a dev dependency?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, because the method randomHomeworkViewWith is inside the normal code for a fake "new" teacher bloc with random states that is still left inside the code. I wouldn't want to remove with this PR, so this usage is still present and thus I need to have it in the normal dependencies.


dev_dependencies:
async: ^2.11.0
Expand Down
14 changes: 2 additions & 12 deletions lib/hausaufgabenheft_logik/test/create_homework_util.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,11 @@
//
// SPDX-License-Identifier: EUPL-1.2

import 'dart:math';

import 'package:common_domain_models/common_domain_models.dart';
import 'package:hausaufgabenheft_logik/src/models/homework/homework.dart';
import 'package:hausaufgabenheft_logik/src/models/homework/models_used_by_homework.dart';
import 'package:hausaufgabenheft_logik/src/views/color.dart';
import 'package:test_randomness/test_randomness.dart';

HomeworkReadModel createHomework(
{Date todoDate = const Date(day: 1, month: 1, year: 2019),
Expand All @@ -22,7 +21,7 @@ HomeworkReadModel createHomework(
bool withSubmissions = false,
Color? subjectColor,
String abbreviation = 'Abb'}) {
id = id == 'willBeRandom' ? randomString(5) : id;
id = id == 'willBeRandom' ? randomAlphaNumeric(5) : id;
return HomeworkReadModel(
id: HomeworkId(id),
todoDate: todoDate.asDateTime(),
Expand All @@ -32,12 +31,3 @@ HomeworkReadModel createHomework(
status: done ? CompletionStatus.completed : CompletionStatus.open,
);
}

String randomString(int length) {
var rand = Random();
var codeUnits = List.generate(length, (index) {
return rand.nextInt(33) + 89;
});

return String.fromCharCodes(codeUnits);
}
Loading
Loading