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

Add ios analyzer command for universal links #134155

Merged

auto-submit merged 2 commits into flutter:master from chunhtai:ios-analyze

Sep 8, 2023

Contributor

chunhtai commented Sep 6, 2023 •

edited

ios version of https://github.com/flutter/flutter/pull/131009/files

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, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
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.

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

chunhtai requested a review from christopherfujino

September 6, 2023 17:59

github-actions bot added the tool label

christopherfujino reviewed

View reviewed changes

packages/flutter_tools/lib/src/commands/analyze.dart

    
            @@ -107,6 +109,12 @@ class AnalyzeCommand extends FlutterCommand {
          
                    hide: !verboseHelp,

                  );

                  argParser.addFlag('ios',

Member

christopherfujino Sep 6, 2023

would we ever want to grab the data from both ios and android subprojects at the same time?

Contributor Author

chunhtai Sep 6, 2023

The dashboard would grab both data to display a comprehensive view.

but I imagine there will be case we only want the date for a certain platform.

christopherfujino reviewed

View reviewed changes

packages/flutter_tools/lib/src/commands/analyze.dart Outdated

    
                  argParser.addOption('configuration',

                    help: 'Sets the iOS build configuration to be analyzed.',

                    valueHelp: 'use "flutter analyze --ios --list-build-options" to get '

Member

christopherfujino Sep 6, 2023

So this long string is messing up verbose help formatting (the android --build-variant help also does this, I must not have checked for that PR). Can you make valueHelp shorter, and then move these instructions to the help field? Something like:

valueHelp: '<configuration>',

Member

christopherfujino Sep 6, 2023

Contributor Author

chunhtai Sep 6, 2023

oops I will do

christopherfujino reviewed

View reviewed changes

packages/flutter_tools/lib/src/commands/ios_analyze.dart Show resolved Hide resolved

christopherfujino reviewed

View reviewed changes

packages/flutter_tools/test/commands.shard/hermetic/ios_analyze_test.dart Outdated

		import 'package:test/test.dart';

Member

christopherfujino Sep 6, 2023

common.dart is our test utility library that re-exports what we need from package:test

Suggested change

      
            import 'package:test/test.dart';
          
            import '../../src/common.dart';

christopherfujino reviewed

View reviewed changes

packages/flutter_tools/test/commands.shard/hermetic/ios_analyze_test.dart Outdated

    
                    }

                  });

                  test('can output json file', () async {

Member

christopherfujino Sep 6, 2023

Suggested change

      
                test('can output json file', () async {
          
                testWithoutContext('can output json file', () async {

christopherfujino reviewed

View reviewed changes

packages/flutter_tools/test/commands.shard/hermetic/ios_analyze_test.dart Outdated

    
                    expect(ios.outputTarget, expectedTarget);

                  });

                  test('can list build options', () async {

Member

christopherfujino Sep 6, 2023

Suggested change

      
                test('can list build options', () async {
          
                testWithoutContext('can list build options', () async {

christopherfujino reviewed

View reviewed changes

packages/flutter_tools/test/commands.shard/hermetic/ios_analyze_test.dart Show resolved Hide resolved

christopherfujino reviewed

View reviewed changes

packages/flutter_tools/test/commands.shard/hermetic/ios_analyze_test.dart Show resolved Hide resolved

christopherfujino reviewed

View reviewed changes

packages/flutter_tools/test/commands.shard/hermetic/ios_analyze_test.dart

    
                      target: expectedTarget,

                      logger: logger,

                    ).analyze();

                    expect(logger.statusText, contains(expectedOutputFile));

Member

christopherfujino Sep 6, 2023

i think we should test that we're writing the write data to this file.

Contributor Author

chunhtai Sep 6, 2023

The writing data part is mocked in this test, and the actual functionality is tested in the IosProject.outputUniversalLinkSettings unit test

flutter/packages/flutter_tools/test/general.shard/project_test.dart

Line 727 in cf1968a

testWithMocks('build with flavor', () async {

Contributor Author

chunhtai Sep 6, 2023 •

edited

This test in this pr is just making sure the analyze end up calling IosProject.outputUniversalLinkSettings with the right parameters

Member

christopherfujino Sep 7, 2023

sorry for the delay, I will finish reviewing before EOD today (Thursday)

chunhtai added 2 commits

September 7, 2023 09:54


          Add ios analyzer command for universal links

ac55c76


          addressing comments

b58ceac

chunhtai force-pushed the ios-analyze branch from 9be05c6 to b58ceac Compare

September 7, 2023 16:54

chunhtai requested a review from christopherfujino

September 7, 2023 16:58

christopherfujino approved these changes

View reviewed changes

Member

christopherfujino left a comment

LGTM

chunhtai added the autosubmit label

Contributor

XilaiZhang commented Sep 8, 2023

Oh wow, overriding github status is finally working now. Sorry I override the google testing to pass for this pr, was using this pr to test a fix we have been working on. Definitely let me know if you need an actual Google Testing test to run.

auto-submit bot merged commit f851e7f into flutter:master

120 checks passed

engine-flutter-autoroll mentioned this pull request

Roll Flutter from da676f79aa37 to 3f4ee3fbcc33 (17 revisions) flutter/packages#4876

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request


          f851e7faf Add ios analyzer command for universal links (flutter/flutt…

6f700b6

…er#134155)

engine-flutter-autoroll mentioned this pull request

Roll Flutter from da676f79aa37 to 55558367127f (18 revisions) flutter/packages#4877

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request


          f851e7faf Add ios analyzer command for universal links (flutter/flutt…

c3e4067

…er#134155)

engine-flutter-autoroll mentioned this pull request

Roll Flutter from da676f79aa37 to 6660cc981669 (19 revisions) flutter/packages#4878

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request


          f851e7faf Add ios analyzer command for universal links (flutter/flutt…

9b28167

…er#134155)

engine-flutter-autoroll mentioned this pull request

Roll Flutter from da676f79aa37 to 7c28e8eb0734 (20 revisions) flutter/packages#4879

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request


          f851e7faf Add ios analyzer command for universal links (flutter/flutt…

d21dd2b

…er#134155)

auto-submit bot pushed a commit to flutter/packages that referenced this pull request


          Roll Flutter from da676f79aa37 to 7c28e8eb0734 (20 revisions) (#4879)

e656e79

flutter/flutter@da676f7...7c28e8e

2023-09-09 engine-flutter-autoroll@skia.org Roll Flutter Engine from 4e3231af6efc to d1913cb6a276 (1 revision) (flutter/flutter#134355)
2023-09-09 engine-flutter-autoroll@skia.org Roll Flutter Engine from 348e3a376807 to 4e3231af6efc (1 revision) (flutter/flutter#134353)
2023-09-09 polinach@google.com Day picker should dispose created MaterialStatesController's. (flutter/flutter#133884)
2023-09-09 engine-flutter-autoroll@skia.org Roll Flutter Engine from 00ef109b845e to 348e3a376807 (1 revision) (flutter/flutter#134349)
2023-09-09 engine-flutter-autoroll@skia.org Roll Flutter Engine from 7af8a5d8d556 to 00ef109b845e (1 revision) (flutter/flutter#134336)
2023-09-09 engine-flutter-autoroll@skia.org Roll Flutter Engine from 1f2da3d69da7 to 7af8a5d8d556 (2 revisions) (flutter/flutter#134332)
2023-09-09 engine-flutter-autoroll@skia.org Roll Flutter Engine from 3a5f3ad1d054 to 1f2da3d69da7 (1 revision) (flutter/flutter#134328)
2023-09-09 engine-flutter-autoroll@skia.org Roll Flutter Engine from d6aa2d9061c1 to 3a5f3ad1d054 (3 revisions) (flutter/flutter#134327)
2023-09-09 engine-flutter-autoroll@skia.org Roll Flutter Engine from 66bec85d5005 to d6aa2d9061c1 (1 revision) (flutter/flutter#134324)
2023-09-08 engine-flutter-autoroll@skia.org Roll Flutter Engine from 8d2892211366 to 66bec85d5005 (3 revisions) (flutter/flutter#134321)
2023-09-08 engine-flutter-autoroll@skia.org Roll Flutter Engine from 2da727e23518 to 8d2892211366 (1 revision) (flutter/flutter#134316)
2023-09-08 sokolovskyi.konstantin@gmail.com Fix memory leak in _DraggableScrollableSheetState (flutter/flutter#134212)
2023-09-08 polinach@google.com RestorationManager should dispatch creation in constructor. (flutter/flutter#133911)
2023-09-08 31859944+LongCatIsLooong@users.noreply.github.com Remove TextPainter migration flag from the framework (flutter/flutter#134274)
2023-09-08 rmolivares@renzo-olivares.dev InputDecoration.error should activate error state (flutter/flutter#134001)
2023-09-08 engine-flutter-autoroll@skia.org Roll Flutter Engine from b2cb1d271a88 to 2da727e23518 (1 revision) (flutter/flutter#134314)
2023-09-08 tessertaha@gmail.com Update chip docs to clarify how to specify a shape with no border & explain default values for Material 3 (flutter/flutter#134298)
2023-09-08 47866232+chunhtai@users.noreply.github.com Add ios analyzer command for universal links (flutter/flutter#134155)
2023-09-08 engine-flutter-autoroll@skia.org Roll Flutter Engine from 47a79306eed3 to b2cb1d271a88 (5 revisions) (flutter/flutter#134313)
2023-09-08 engine-flutter-autoroll@skia.org Roll Flutter Engine from 6d6b44886175 to 47a79306eed3 (2 revisions) (flutter/flutter#134310)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC rmistry@google.com,stuartmorgan@google.com,tarrinneal@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request


          f851e7faf Add ios analyzer command for universal links (flutter/flutt…

c9926c2

…er#134155)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment