Uh oh!
There was an error while loading. Please reload this page.
run_ios_tests: Allow running from any directory - #190822
Conversation
Previously `run_ios_tests.sh` only worked when invoked from `engine/src/flutter` which, to be fair, is what the READMEs tell you to do but there's no reason we need to force that, and we don't for other tests like run_tests.py. This is mostly just post-monorepo-merge cleanup. Two separate things depended on the working directory: * The wrapper was resolving `SCRIPT_DIR` from `BASH_SOURCE` but still passed the Dart entrypoint as a relative path, so the VM would report `No such file or directory`. * `run_ios_tests.dart` called `Engine.tryFindWithin()`, which defaults to the current directory and only walks upward. From the repo root `engine/src` is below the starting point, so the search fails and the script exits with `Must be run from within the engine repository.` Both now resolve from the script's own location, so the documented invocation keeps working and but invoking from any other directory works too. The Dart side matches the existing usage in `tools/header_guard_check/lib/header_guard_check.dart:162`. Also fixed up the READMEs, which had a few other issues... one if which was that I forgot to update them in flutter#190818.
It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
There was a problem hiding this comment.
Code Review
This pull request updates the iOS scenario app testing scripts and documentation to allow running the tests from any directory. Specifically, 'run_ios_tests.dart' now searches for the engine repository starting from its own script location rather than the current working directory, and 'run_ios_tests.sh' uses the script's directory to locate the Dart entry point. Additionally, related documentation and paths in 'README.md' files are updated to reflect these changes and the transition from 'AppDelegate' to 'SceneDelegate' for scenario arguments. There are no review comments, so no further feedback is provided.
hellohuanlin
left a comment
There was a problem hiding this comment.
thanks for fixing the readme files too
Previously
run_ios_tests.shonly worked when invoked fromengine/src/flutterwhich, to be fair, is what the READMEs tell you to do but there's no reason we need to force that, and we don't for other tests like run_tests.py. This is mostly just post-monorepo-merge cleanup.Two separate things depended on the working directory:
SCRIPT_DIRfromBASH_SOURCEbut still passed the Dart entrypoint as a relative path, so the VM would reportNo such file or directory.run_ios_tests.dartcalledEngine.tryFindWithin(), which defaults to the current directory and only walks upward. From the repo rootengine/srcis below the starting point, so the search fails and the script exits withMust be run from within the engine repository.Both now resolve from the script's own location, so the documented invocation keeps working and but invoking from any other directory works too. The Dart side matches the existing usage in
tools/header_guard_check/lib/header_guard_check.dart:162.Also fixed up the READMEs, which had a few other issues... one if which was that I forgot to update them in #190818.
No test changes because this is test changes.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.