Skip to content

Conversation

stuartmorgan-g
Copy link
Collaborator

  • Consolidates list.dart and async_handlers.dart into core_tests.dart. This involved adding a couple of new APIs that I expect to be temporary; it's not ideal, but it's simpler (and now more clearly commented) than having random extra pigeon input files.
  • Eliminates android_unittest.dart; some of the tests using it have been removed as they are not useful relative to the integration tests we now run (e.g., ensuring that serialization and deserialization of a data class works without asserting anything about its structure), and the others have been adjusted to use the new core_test.dart API instead.
  • Eliminates several files that were not used in any tests, and were only having Dart analysis run; they don't cover anything that isn't already in core_tests.dart.

This is just a natural cut point since the remaining files are either more complex and/or are also used in the Dart unit test project that itself needs some rework; more still needs to be done to remove more files.

Part of flutter/flutter#115168

Pre-launch Checklist

  • 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 relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
  • I signed the CLA.
  • The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
  • I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

@tarrinneal
Copy link
Contributor

Cannot find source file:

pigeon/async_handlers.gen.cpp

seems a test is still in need of updating for windows

@stuartmorgan-g
Copy link
Collaborator Author

Yep, I'm already on it. I meant to check that before I opened the PR, but forgot overnight.

@stuartmorgan-g
Copy link
Collaborator Author

No actual test updates needed, I just had to remove references to the generated output of some files. I enabled builds for almost everything on Windows even where I didn't backfill unit tests since it meant we at least had compilation coverage.

/**
* A data class containing a List, used in unit tests.
*
* <p>Generated class from Pigeon that represents data sent in messages.
Copy link
Contributor

@tarrinneal tarrinneal Feb 16, 2023

Choose a reason for hiding this comment

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

where is this <p> coming from?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If we're not generating it, I guess the autoformatter?

Copy link
Contributor

Choose a reason for hiding this comment

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

I just looked into it and it seems to be standard practice for doc comment paragraph separation. Might be a good idea to add it to our comment parsing tools

@tarrinneal tarrinneal added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 16, 2023
@auto-submit auto-submit bot merged commit 763d025 into flutter:main Feb 16, 2023
@stuartmorgan-g stuartmorgan-g deleted the pigeon-more-test-consolidation branch February 17, 2023 01:47
ditman added a commit to ditman/flutter-flutter that referenced this pull request Feb 24, 2023
* ad48ee5 [go_router] Fix some broken links in doc (flutter/packages#3288)
* 58ac45e [go_router_builder] Add support for Iterable, List  and Set to TypedGoRoute (flutter/packages#2679)
* 0edae25 Export super types in route_data.dart library (flutter/packages#3286)
* 8ad3fde [go_router] Add `GoRouter.maybeOf` (flutter/packages#3216)
* 13ee644 [flutter_migrate] Skip slow tests (flutter/packages#3270)
* 3cc754a Update .gitignore with missing values from flutter/plugins (flutter/packages#3265)
* c0f0a22 [ci] Re-enable pathified unit tests (flutter/packages#3268)
* 5834b4c [go_router]: implemented helpers for ShellRoute (flutter/packages#2730)
* af5906b [extension_gsi] Update extension to support gsi 5 and 6. (flutter/packages#3235)
* 195f4e8 Merge in plugin README and CONTRIBUTING (flutter/packages#3252)
* fab47af [go_router] Disable logging in tests (flutter/packages#3263)
* 25f0f70 [various] Update flutter/plugins links (flutter/packages#3256)
* 2e16733 Merge flutter/plugins (flutter/packages#3233)
* 324a7f2 Exclude more tests on Windows
* 334b58e Adjust test configs
* 69e6dac [go_router_builder] Generate replace method in RouteExtension (flutter/packages#2838)
* 193e454 Merge repository metadata
* 18715d7 Merge remote-tracking branch 'plugins-packages/main' into merge-flutter-plugins
* f2d802d Roll Flutter from ae8d051 to 7175de4 (4 revisions) (flutter/packages#3232)
* 6f1b1e8 Roll Flutter from 33e4d21 to ae8d051 (6 revisions) (flutter/packages#3229)
* a162a98 Roll Flutter from 0be7c3f to 33e4d21 (5 revisions) (flutter/packages#3227)
* ab5a8c0 [tool] Allow importing packages with NEXT (flutter/packages#3215)
* ce9c61b Roll Flutter from 170539f to 0be7c3f (38 revisions) (flutter/packages#3225)
* 9747469 Fix deprecation message for GoRouterState.namedLocation (flutter/packages#3092)
* 6e4431f [go_router] Bump example `compileSdkVersion` and `package_info_plus` dependency version (flutter/packages#3219)
* 925bea8 [pigeon] Validate generated files in CI (flutter/packages#3224)
* 3094867 Move iOS Swift unit tests back to Cirrus (flutter/packages#3221)
* 763d025 [pigeon] Eliminate some of the test pigeons (flutter/packages#3213)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App p: pigeon
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants