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.

Sign up for GitHub

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

Jump to bottom

Mock list all #33

Merged
merged 5 commits into from
Feb 28, 2023
Merged

Mock list all #33

merged 5 commits into from
Feb 28, 2023

Conversation

saroad2
Copy link
Contributor

@saroad2 saroad2 commented Feb 21, 2023

An implementation of the function listAll for MockReference

Related to #32

@saroad2
Copy link
Contributor Author

saroad2 commented Feb 25, 2023

Hey @atn832 , would love your feedback on this!
My application tests are failing due to the lack of this functionality 🥺

atn832
atn832 reviewed Feb 28, 2023
View reviewed changes
test/firebase_storage_mocks_test.dart
});
}

void expectRef(Reference actualReference, {required String name}) {
Copy link
Owner

Choose a reason for hiding this comment

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

Nice!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you!

atn832
atn832 reviewed Feb 28, 2023
View reviewed changes
test/firebase_storage_mocks_test.dart
isA<MockReference>().having((ref) => ref.name, 'Right name', name));
}

Uint8List randomData(int n) {
Copy link
Owner

Choose a reason for hiding this comment

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

Nice!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you!

@atn832
Copy link
Owner

atn832 commented Feb 28, 2023

Looks good, would you mind applying the 2 small changes?

@saroad2
Copy link
Contributor Author

saroad2 commented Feb 28, 2023

Hey @atn832 , fixed everything you mentioned in the review. Please let me know if there is something elses needed to be done.

Nevertheless, I would really appriciate it if you could release a new version of firebase_storage_mock after you merge this PR🙏 .

atn832
atn832 reviewed Feb 28, 2023
View reviewed changes
test/firebase_storage_mocks_test.dart
final basePath = 'this/is/basic';
final otherPath = 'another/path';
final storage = MockFirebaseStorage();
await storage.ref(basePath + '/listed_data').putData(randomData(255));
Copy link
Owner

Choose a reason for hiding this comment

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

Much better, thank you!

@atn832 atn832 merged commit 02325aa into atn832:master Feb 28, 2023
@saroad2 saroad2 deleted the mock_list_all branch February 28, 2023 23:23
@atn832
Copy link
Owner

atn832 commented Feb 28, 2023

Nevertheless, I would really appriciate it if you could release a new version of firebase_storage_mock after you merge this PR🙏 .

Sure thing. I just published your feature to https://pub.dev/packages/firebase_storage_mocks/changelog#061. Thanks for the high quality PR.

Btw, in the future, if your work depends urgently on your fixes, you can temporarily change your yaml file to depend on your fork. See https://dart.dev/tools/pub/dependencies#git-packages.

@atn832 atn832 mentioned this pull request Mar 13, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@atn832 atn832 atn832 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@saroad2 @atn832