[pigeon] replaced all_datatype compilation tests with platform unit tests

[gaaclarke]

issue: flutter/flutter#84155

Pre-launch Checklist

• The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene wiki page, which explains my responsibilities.
• I read and followed the Flutter Style Guide.
• I listed at least one issue that this PR fixes in the description above.
• I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt.
• I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
• I updated CHANGELOG.md to add a description of the change.
• I updated/added relevant documentation (doc comments with ///).
• I signed the CLA.
• All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

[gaaclarke]

@stuartmorgan Looks like we've added a license check to CI. What do we want to do about generated code? Add an exception? Make Pigeon generate a custom header? It seems like we want the warning about being autogenerated on top even if we add a copyright.

Here's the error:

The license block for these files is missing or incorrect:
  /tmp/cirrus-ci-build/packages/pigeon/platform_tests/android_unit_tests/android/app/src/main/java/com/example/android_unit_tests/AllDatatypes.java
  /tmp/cirrus-ci-build/packages/pigeon/platform_tests/flutter_null_safe_unit_tests/lib/all_datatypes.dart
  /tmp/cirrus-ci-build/packages/pigeon/platform_tests/flutter_null_safe_unit_tests/regenerate_mocks.sh
  /tmp/cirrus-ci-build/packages/pigeon/platform_tests/ios_unit_tests/ios/Runner/all_datatypes.h
  /tmp/cirrus-ci-build/packages/pigeon/platform_tests/ios_unit_tests/ios/Runner/all_datatypes.m
  /tmp/cirrus-ci-build/packages/pigeon/platform_tests/ios_unit_tests/ios/Runner/async_handlers.h
  /tmp/cirrus-ci-build/packages/pigeon/platform_tests/ios_unit_tests/ios/Runner/async_handlers.m
If this third-party code, move it to a "third_party/" directory, otherwise ensure that you are using the exact copyright and license text used by all first-party files in this repository.

[stuartmorgan-g]

What do we want to do about generated code?

When I fixed everything in order to land the license check I just manually added it to the Pigeon files, under the theory that re-generating them is not likely to be something we do frequently so it wouldn't be a problem to just do that again next time.

Add an exception?

No, the guidance we have received is that all code we check in should have a license block.

Make Pigeon generate a custom header?

Having Pigeon generate a header would work as well. Presumably that would be something like an optional file path containing the content to put at the top of each file.

It seems like we want the warning about being autogenerated on top even if we add a copyright.

Maybe? Usually warnings like that are in files that are auto-generated as part of a build system (meaning changes will either always be destroyed, or be unpredictably destroyed, depending on the details), which isn't the case with Pigeon. I'm not against it being there, but given that it requires manual generation I'm not sure there's really much argument against someone editing them; yes, they could stomp their changes the next time the manually re-generate the file, but that seems pretty obviously what would happen.

Edit Oh, you mean at the very top. No, the license block should always be before any file comments.

[stuartmorgan-g]

Not for this PR, but it would be useful to look into whether the native unit tests could be using flutter_plugin_tools, which has pretty similar logic for running native unit tests for plugins.

[stuartmorgan-g]

The code LG, but are the Android unit tests actually running in CI? I looked at the output, and I see output from the ObjC tests, but couldn't find anything that looked like output from the Java unit tests. Maybe I just missed it, but it seems worth verifying.

[gaaclarke]

The code LG, but are the Android unit tests actually running in CI? I looked at the output, and I see output from the ObjC tests, but couldn't find anything that looked like output from the Java unit tests. Maybe I just missed it, but it seems worth verifying.

Yep, CI is executing "run_tests.sh", the same thing I use when developing.

[stuartmorgan-g]

LGTM modulo looking at the CI output to make sure you are seeing the Android test results.

Sorry for the delay, I thought you were going to update this to split apart the tests in Cirrus, but maybe you meant that to be a follow-up.

[gaaclarke]

The output from the tests is a common complaint. I tried the options suggested in this SO question. The -i seemed to print out tons of worthless info and the afterTest closures didn't seem to print out anything:

diff --git a/packages/pigeon/platform_tests/android_unit_tests/android/app/build.gradle b/packages/pigeon/platform_tests/android_unit_tests/android/app/build.gradle
index f45bc0b..204eb22 100644
--- a/packages/pigeon/platform_tests/android_unit_tests/android/app/build.gradle
+++ b/packages/pigeon/platform_tests/android_unit_tests/android/app/build.gradle
@@ -51,6 +51,11 @@ android {
 
     testOptions {
         unitTests.returnDefaultValues = true
+        unitTests.all {
+            afterTest { descriptor, result ->
+                println "Executing test for ${descriptor.name} with result: ${result.resultType}"
+            }
+        }
     }
 }

[gaaclarke]

Sorry for the delay, I thought you were going to update this to split apart the tests in Cirrus, but maybe you meant that to be a follow-up.

Yep, thanks for the reminder on that too.