8000 Move android_obfuscate_test from devicelab into tools integration.shard by jmagman · Pull Request #169798 · flutter/flutter · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Move android_obfuscate_test from devicelab into tools integration.shard #169798

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Move android_obfuscate_test from devicelab into tools integration.shard #169798

wants to merge 3 commits into from
+118 −123

Conversation

jmagman
Copy link
Member
@jmagman jmagman commented May 30, 2025 •
edited
Loading

android_obfuscate_test is currently being schedule to run on a limited devicelab physical phone bot. However, it's only checking if --obfuscate works, not running anything on the device, so can move into the tools test integration shard.

https://ci.chromium.org/ui/p/flutter/builders/prod/Linux_pixel_7pro%20android_obfuscate_test/7722/overview

The test only takes < 2 minutes, spinning up an entire build for it, especially on a devicelab bot, is wasteful.

Similar to #80161

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@github-actions github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label May 30, 2025
jmagman
jmagman commented May 30, 2025
View reviewed changes
packages/flutter_tools/test/integration.shard/android_obfuscate_test.dart
}

bool _containsSymbol(File outputArchive, String libappPath, String symbol) {
final Archive archive = ZipDecoder().decodeBytes(outputArchive.readAsBytesSync());
Copy link
Member Author

Choose a reason for hiding this comment

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

A better version of

flutter/packages/flutter_tools/test/host_cross_arch.shard/ios_content_validation_test.dart

Lines 278 to 298 in 11bf180

testWithoutContext('validate obfuscation', () {
// HelloPlugin class is present in project.
ProcessResult grepResult = processManager.runSync(<String>[
'grep',
'-r',
'HelloPlugin',
pluginRoot.path,
]);
// Matches exits 0.
expect(grepResult.exitCode, 0);
// Not present in binary.
grepResult = processManager.runSync(<String>[
'grep',
'HelloPlugin',
outputAppFrameworkBinary.path,
]);
// Does not match exits 1.
expect(grepResult.exitCode, 1);
});
});

Inspired by

flutter/packages/flutter_tools/test/integration.shard/deferred_components_test.dart

Lines 61 to 65 in 11bf180

final Archive archive = ZipDecoder().decodeBytes(outputFile.readAsBytesSync());
expect(archive.findFile('base/lib/arm64-v8a/libapp.so') != null, true);
expect(archive.findFile('base/lib/arm64-v8a/libflutter.so') != null, true);
expect(archive.findFile('component1/lib/arm64-v8a/libapp.so-2.part.so') != null, true);

packages/flutter_tools/test/integration.shard/android_obfuscate_test.dart
'build',
'apk',
'--target-platform=android-arm',
'--obfuscate',
Copy link
Member Author

Choose a reason for hiding this comment

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

I confirmed both of these tests fail with --no-obfuscate passed instead.

jmagman
jmagman commented May 30, 2025
View reviewed changes
dev/devicelab/bin/tasks/android_obfuscate_test.dart Outdated

Future<void> main() async {
// This test has moved to packages/flutter_tools/test/integration.shard/android_obfuscate_test.dart.
// TODO(jmagman): Remove this file once the infra repo no longer contains "Linux_pixel_7pro android_obfuscate_test".
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm unsure if this extra step is necessary, or I can remove this file in the same PR.

@jmagman jmagman marked this pull request as ready for review June 13, 2025 16:55
@jmagman jmagman requested a review from matanlurey as a code owner June 13, 2025 16:55
@jmagman jmagman requested a review from a team June 13, 2025 16:55
@matanlurey matanlurey mentioned this pull request Jun 16, 2025
Closed
@jmagman jmagman added the platform-android Android applications specifically label Jun 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matanlurey matanlurey matanlurey approved these changes

Assignees
No one assigned
Labels
platform-android Android applications specifically tool Affects the "flutter" command-line tool. See also t: labels.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jmagman @matanlurey
0