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

Conversation

@dnfield
Copy link
Contributor

@dnfield dnfield commented Jul 12, 2021

See discussion in #27332

This target used to get built by default in run_tests.py, but we avoid doing that now for CI

Once this lands, we can remove the separate build step(s) for it from CI.

@dnfield
Copy link
Contributor Author

dnfield commented Jul 12, 2021

Ahhh this doesn't work.

@dnfield
Copy link
Contributor Author

dnfield commented Jul 12, 2021

The test target was depending on the top level android target rather than :android_jar which is what it really needs.

deps += [ ":flutter_shell_native" ]
}

if (flutter_runtime_mode == "debug" && current_cpu == "arm") {
Copy link

Choose a reason for hiding this comment

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

does the current cpu matter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This target is relatively slow to build, and we only run these tests on android_debug_unopt (armv7).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you mean current v. target, this file refers to current elsewhere and it seems like current == target .. but current is not the "host" CPU

Copy link

Choose a reason for hiding this comment

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

I see. could we add a comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link

@blasten blasten left a comment

Choose a reason for hiding this comment

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

LGTM % comment

@dnfield dnfield added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Jul 12, 2021
@fluttergithubbot
Copy link
Contributor

This pull request is not suitable for automatic merging in its current state.

@fluttergithubbot fluttergithubbot removed the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Jul 13, 2021
@dnfield
Copy link
Contributor Author

dnfield commented Jul 13, 2021

Blocked on flutter/flutter#86326

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.

4 participants