Skip to content

[Flutter Driver] Simplified the serialization/deserialization logic of the Descendant/…#40715

Merged
adazh merged 3 commits intoflutter:masterfrom
adazh:descendant_matcher
Sep 18, 2019
Merged

[Flutter Driver] Simplified the serialization/deserialization logic of the Descendant/…#40715
adazh merged 3 commits intoflutter:masterfrom
adazh:descendant_matcher

Conversation

@adazh
Copy link
Copy Markdown
Contributor

@adazh adazh commented Sep 17, 2019

…Ancestor matchers.

Instead of flattening the "of" and "matching" matchers to the JSON string, using jsonEncode/jsonDecode to convert the matchers to String/from String.

This will make the serialization in Espresso/Java much simpler.

Description

Updated the serialization/deserialization logic for the Descendant/Ancestor matchers. Take the "get_diagnostics_tree" for example.
Before:
{"id":"message-7","method":"ext.flutter.driver","params":{"isolateId":"isolates/4120307314110843","of_keyValueType":"String","of_keyValueString":"TwoTimesCounterContainer","of_finderType":"ByValueKey","matching_type":"Text", "matcher_finderType":
"ByType","finderType":"Descendant"
,"diagnosticsType":"widget","includeProperties":true,"subtreeDepth":20,"command":"get_diagnostics_tree","timeout":10000},"jsonrpc":"2.0"}

After:
{"id":"message-7","method":"ext.flutter.driver","params":{"isolateId":"isolates/4120307314110843","of":"{\"keyValueType\":\"String\",\"keyValueString\":\"TwoTimesCounterContainer\",\"finderType\":\"ByValueKey\"}","matching":"{
\"type\":\"Text\",\"finderType
":\"ByType\"}"
,"finderType":"Descendant","diagnosticsType":"widget","includeProperties":true,"subtreeDepth":20,"command":"get_diagnostics_tree","timeout":10000},"jsonrpc":"2.0"}

Tests

flutter_test test/src/extension_test.dart still passes.

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.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I signed the CLA.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require Flutter developers to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (Please read Handling breaking changes). Replace this with a link to the e-mail where you asked for input on this proposed change.
  • No, this is not a breaking change.

@fluttergithubbot
Copy link
Copy Markdown
Contributor

It looks like this pull request may not have tests. Please make sure to add tests before merging. While there are exceptions to this rule, if this patch modifies code it is probably not an exception.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@fluttergithubbot fluttergithubbot added a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels. labels Sep 17, 2019
@adazh adazh requested a review from goderbauer September 17, 2019 23:40
@goderbauer
Copy link
Copy Markdown
Member

This didn't require any changes to tests?

@adazh
Copy link
Copy Markdown
Contributor Author

adazh commented Sep 17, 2019

Ah, forgot about the find_test.dart. On it.

@adazh
Copy link
Copy Markdown
Contributor Author

adazh commented Sep 17, 2019

This didn't require any changes to tests?

Tests are updated.

Copy link
Copy Markdown
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

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

LGTM

Technically, this is a breaking change for anybody using the API, but that's probably just flutter driver and you?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: indentation.

@adazh
Copy link
Copy Markdown
Contributor Author

adazh commented Sep 18, 2019

LGTM

Technically, this is a breaking change for anybody using the API, but that's probably just flutter driver and you?

Yep, I think so.

@adazh adazh force-pushed the descendant_matcher branch from fb7bdd4 to 5c176b8 Compare September 18, 2019 22:25
@adazh adazh merged commit c4016aa into flutter:master Sep 18, 2019
@adazh adazh deleted the descendant_matcher branch September 18, 2019 23:58
Inconnu08 pushed a commit to Inconnu08/flutter that referenced this pull request Sep 30, 2019
…f the Descendant/… (flutter#40715)

* Simplified the serialization/deserialization logic of the Descendant/Ancestor matchers
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Aug 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants