[Pigeon]: version 0.1.0 (objc and testing features)

[gaaclarke]

• cleaned up objc generation
• added ability to generate mock handlers
• added pigeon.dart

[gaaclarke]

This file gets regenerated when running run_tests.sh. I still don't have the time to invest in creating a mechanism to generate these during standard execution of the tests.

[xster]

General approach LGTM. The naming and docs need some more tweaking.

[xster]

Can we make this a bit more explicit? Since this is our canonical code doc that people can cmd+click into when they see the annotation.

Something like

Annotates a platform implemented Pigeon API class.
This class groups a collection of Dart↔platform interop methods. These methods are to be invoked by a Dart-side program and are to be received by a platform-side program (such as in Android or iOS) by an implementation class inheriting this class. 

or some such?

[gaaclarke]

done

[xster]

Here and below, we should be careful with the terminology to avoid confusion. What you're adding here isn't quite a mock which is a stand-in for a real object and that just registers calls. What this PR does doesn't replace the real object. It just adds a matched object that "works" and that's different from the real implementation. i.e. it's a fake.

[gaaclarke]

There is merit to what you are saying but mock is the terminology used by channels, that's why I chose it (https://api.flutter.dev/flutter/services/MethodChannel/setMockMethodCallHandler.html)

[xster]

Now this creates a second class which is exactly the same as the FlutterSearchApi class a bit further down in this file which will make people ask why there's 2 ways of doing the same thing. This file should have more docs.

[gaaclarke]

Not exactly the same but close. I added a comment at the top.

[xster]

Let's not call it mock since it's not mocking Api and ApiSetup. If you're still using the real Api and using this as the receiving side, let's just call it ApiTestHandler in this example.

[gaaclarke]

"mock handler" is already used terminology for the same concept elsewhere: https://api.flutter.dev/flutter/services/MethodChannel/setMockMethodCallHandler.html

[xster]

Ah I see where you're going. That thing is a mock though. It's taking a production class and swapping it out for an alternate one which records and stubs interactions.

This doesn't replace a production class. It's used in parallel with the production class and is a separate "working" class that has a different behavior than the real one.

It'd be like we had client and server and we want to test client. We can have a mockClient which is what we all want but since we can't do it easily for existing tests, we're using client with a fakeServer. But if we do, we can't make the fakeServer be named mockClient. That would be really confusing.

We can call this TestHost[name of API] for instance. Since we're specifically doing the 'host' part of the class annotated by HostApi

[gaaclarke]

I renamed this class to TestHostApi.

[xster]

While we're here, can we fix this and the normal static setup method to use camel case naming?

[gaaclarke]

Changed it to MockApi.setup().

[xster]

oh I like it. What about the production class?

[gaaclarke]

Yep, same.

[xster]

for this specific static method, it should be a verb rather than a noun. Perhaps flip the naming to put setup in front. e.g. setupApiTestHandler.

[gaaclarke]

Changed it to MockApi.setup().

[xster]

I like it

[xster]

Add some more details. Such as

This interface will be placed along side the [HostApi] class. You can implement this
interface and invoke `setup[name of this test handler]` to receive calls from your real [HostApi] class in Dart instead of the host platform code as is the case typically. Generally, opt to use a mock of the real [HostApi], such as via mockito, in unit testing. But generating this Dart side test handler could sometimes be useful in integration testing. Defaults to `null` in which case no test handler will be generated.

[gaaclarke]

Done.

[xster]

dartTestHandler

[gaaclarke]

as noted above

[xster]

I'd still call this dartHostTestHandler

[gaaclarke]

@xster I think I addressed all the comments. There may still be a question about mockDartHandler let me know if that's still the case and what you decided you'd like it to be. It will be a bit of a pain to change it and want to make sure to get it right.

[xster]

lgtm % the field name on the annotation

[xster]

great! We should note the need to call this in the package's README

[xster]

I'm a bit surprised though that it's in this abstract FlutterSearchApi class rather than being in Api.

[xster]

Not for this PR but I don' think we should do it soon. We should doc annotate the output classes, especially since we decided that the person writing the pigeon file might not be the person consuming these Dart/Java/ObjC classes and the latter could use some help in knowing what to do with these.

[xster]

nit: here and elsewhere, there's a bunch of double spaces.

[xster]

I'd still call this dartHostTestHandler

[xster]

if you rename it, keep the name consistent here and below

[gaaclarke]

Landing before build-ipa's task finish. They seem to be fubar, saying stuff about upgrading xcode but showing no progress. Those tests previously passed before my renaming of the field.