Switch existings tests to use equalsIgnoringHashCodes and add more tests. #10954
Conversation
There was a problem hiding this comment.
can you add a TODO(ianh) to fix this?
There was a problem hiding this comment.
Added TODO(jacobr) because I'm about to send you a CL that incidentally fixes this issue.
|
LGTM. I'm fine with checking this in. I think we would be better served by specific tests of each class, though. |
|
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the |
jacob314
force-pushed
the
review_add_tests
branch
from
4782765 to
a4b6e23
Compare
|
CLAs look good, thanks! |
There was a problem hiding this comment.
I had to reduce this test to a shallow test of properties due to the platform dependent toStringDeep text issue described above.
OI could also change
equalsIgnoringHashCodes
to also normalize platform names but
I would have to change its name as well if I did that.
There was a problem hiding this comment.
aren't you going to have the same issue here?
|
You can just force the platform to be whatever you want it to be, I think. Doesn't it come from the Theme? |
jacob314
force-pushed
the
review_add_tests
branch
from
a4b6e23 to
648e2ad
Compare
|
Fixed the issue that was causing the platform to not match the platform specified for tests in a separate CL. |
| expect(didBuild, isTrue); | ||
| final RenderObject theater = overlayKey.currentContext.findRenderObject(); | ||
|
|
||
| // toStringDeep output is missing a trailing line break. |
|
|
…sts. (flutter#10954) * Switch existings tests to use equalsIgnoringHashCodes and add more tests.
3 participants
I'm open to keeping some of these tests on a local branch if we are concerned they will cause
too many busy work changes as widgets are updated. Just let me know which tests to keep out of the repo.
These tests were useful in refactoring the toStringDeep code but are perhaps a bit excessive
outside of that context.