Skip to content

Conversation

@stuartmorgan-g
Copy link
Collaborator

This is an interim solution to get the ObjC tests into the new combined harness while the CI blocking issues in #2816 are being resolved. This is based on that PR, with the following changes:

  • platform_tests/unit_tests_ios/ is restored, but without any of the source files; the project has been modified to point to the locations of those files in the new harness.
    • Using the tests in two different locations required hacking the imports a bit since the new, simpler, module-based import doesn't work in the old location (since the sources aren't in an alternate_language_test_plugin module there), so there's an #ifdef-based inclusion of the old approach for now.
  • The ios_unittests test target in run_tests.sh has been changed back to running the old harness, so that CI will continue to work as is.
  • A new ios_objc_unittests target in run_tests.sh (which is the name we actually want going forward) has been added, and like the other new tests is just a passthrough to a new run_tests.dart implementation.

This will allow us to land the migration to the new harness now, which will avoid the problem of that PR constantly becoming outdated as we make changes in the new harness, while leaving the old harness in place for CI until the CI problems with ARM Cirrus can be resolved (which could be another month), at the cost of having duplicate harnesses for now. Since it's only the project that's duplicated though, not the source, we'll only have to touch the old harness if we are actually changing the included files (most likely that would just be pigeons/ consolidation PRs), which should be a small cost.

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.

@stuartmorgan-g stuartmorgan-g added override: no versioning needed Override the check requiring version bumps for most changes override: no changelog needed Override the check requiring CHANGELOG updates for most changes labels Nov 30, 2022
@stuartmorgan-g
Copy link
Collaborator Author

(Once this lands, I'll rebase #2816 and it should become all deletion, so it won't keep getting stale.)

@tarrinneal tarrinneal added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 30, 2022
@tarrinneal tarrinneal merged commit e1dc854 into flutter:main Nov 30, 2022
@stuartmorgan-g stuartmorgan-g deleted the pigeon-test-new-harness-ios-objc-non-default branch December 1, 2022 01:12
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 override: no changelog needed Override the check requiring CHANGELOG updates for most changes override: no versioning needed Override the check requiring version bumps for most changes p: pigeon

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants