Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Add unwrap to JSONUtil#4491

Merged
mehmetf merged 1 commit intomasterfrom
mehmetf-jsonutil
Jan 6, 2018
Merged

Add unwrap to JSONUtil#4491
mehmetf merged 1 commit intomasterfrom
mehmetf-jsonutil

Conversation

@mehmetf
Copy link
Contributor

@mehmetf mehmetf commented Dec 22, 2017

We have a use for this for the internal messaging plugin. Instead of rolling our own, it made sense to add it here especially since it has the mirror functionality (wrap).

We have a use for this for the internal messaging plugin. Instead of rolling our own, it made sense to add it here especially since it has the mirror functionality (wrap).
@mehmetf
Copy link
Contributor Author

mehmetf commented Dec 22, 2017

Did not find any tests for JSONUtil anywhere. If you point me to it, happy to add tests for this.

@Hixie
Copy link
Contributor

Hixie commented Jan 4, 2018

@mehmetf You may have to blaze new trails in testing to test this code.

@mehmetf
Copy link
Contributor Author

mehmetf commented Jan 6, 2018

@Hixie I do want to test this code; unfortunately I don't have much time to spend on it. Do you have a rough sketch in your mind on how the infra should look like? I can attempt to do it.

Having said that I don't think all that setup should be part of this PR so I will submit this as is and file a PR to create a testing harness for platform code in engine. Feel free to do a brain dump there.

@mehmetf mehmetf merged commit d43ae0b into master Jan 6, 2018
@mehmetf mehmetf deleted the mehmetf-jsonutil branch January 6, 2018 22:32
@aam
Copy link
Member

aam commented Jan 7, 2018

This broke the Linux and Mac builds https://build.chromium.org/p/client.flutter/console :

../../flutter/shell/platform/android/io/flutter/plugin/common/JSONUtil.java:47: error: cannot find symbol
                Iterator<String> keyIterator = jsonObject.keys();
                ^
  symbol:   class Iterator
  location: class JSONUtil
1 error

mehmetf added a commit that referenced this pull request Jan 7, 2018
@Hixie
Copy link
Contributor

Hixie commented Jan 7, 2018

@mehmetf I'm not familiar with how Java normally gets unit-tested, but I would expect it to follow a pattern similar to the C++ or Dart testing for the engine, which is in https://github.com/flutter/engine/tree/master/testing, driven from run_tests.sh.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants