-
Notifications
You must be signed in to change notification settings - Fork 9.7k
[firebase_performance] Testing of firebase performance rewrite PRs #1557
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change LGTM. Minor nit though, I think that assigning test data to variables in unit tests is discouraged and that we're encouraging less code re-use in favor of making the tests more declarative. For example, instead of assigning value to 'apple' you could say 'apple' everywhere.
@amirh may have more opinions about this. I realize this guideline is a bit unintuitive and I don't feel strongly about it, probably we should write it down either way.
I was thinking about this as well. I typically always use literals, but I wanted to make this PR a little shorter and be focused on the refactor while the next one will be focused on tests. |
Description
Reformat
firebase_performance
plugin to enable the use of meaningful integration testing.Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]
). This will ensure a smooth and quick review process.///
).flutter analyze
) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?