Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@jmagman
Copy link
Member

@jmagman jmagman commented Jul 8, 2022

Just building the ios_debug_sim_unopt does not generate the scenario_app. Add instructions to the README to also build the scenario_app target.

See #27302

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 Flutter Style Guide and the C++, Objective-C, Java style guides.
  • 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. See testing the engine for instructions on
    writing and running engine tests.
  • 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.

@jmagman jmagman self-assigned this Jul 8, 2022
@jmagman
Copy link
Member Author

jmagman commented Jul 8, 2022

Just building ios_debug_sim_unopt ios_test_flutter didn't generate out/ios_debug_sim_unopt/scenario_app.
@dnfield I don't see where this is done on the recipe side, am I missing something here?

https://flutter-review.googlesource.com/c/recipes/+/15820

@dnfield
Copy link
Contributor

dnfield commented Jul 8, 2022

It should have - when I test a local build it's working fine, and it should get pulled in by default from the root BUILD.gn here:

engine/BUILD.gn

Lines 133 to 136 in 5f4c237

if ((flutter_runtime_mode == "debug" || flutter_runtime_mode == "profile") &&
(is_ios || is_android)) {
public_deps += [ "//flutter/testing/scenario_app" ]
}

@dnfield
Copy link
Contributor

dnfield commented Jul 8, 2022

explicitly building that target may not be a bad idea because in theory it'll be faster by excluding other targets you may not need, but in practice I doubt it's much different.

So these instructions should not be necessary but they're not wrong either...

@jmagman
Copy link
Member Author

jmagman commented Jul 8, 2022

I see what happened, locally I was running the target ios_test_flutter, which doesn't add the scenario_app dep. Just building ios_debug_sim_unopt on its own (as the README says now) did work.

$ ninja -C out/ios_debug_sim_unopt ios_test_flutter
$ ll out/ios_debug_sim_unopt/scenario_app
ls: out/ios_debug_sim_unopt/scenario_app: No such file or directory
$ ninja -C out/ios_debug_sim_unopt
$ ll out/ios_debug_sim_unopt/scenario_app
total 24
...

Thanks for the sanity check @dnfield.

@jmagman jmagman closed this Jul 8, 2022
@jmagman jmagman deleted the scenario-readme branch July 8, 2022 22:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants