Skip to content

Pin the version of flutter/plugins to test against#71166

Merged
fluttergithubbot merged 2 commits intoflutter:masterfrom
amirh:pin_plugins_version
Nov 25, 2020
Merged

Pin the version of flutter/plugins to test against#71166
fluttergithubbot merged 2 commits intoflutter:masterfrom
amirh:pin_plugins_version

Conversation

@amirh
Copy link
Contributor

@amirh amirh commented Nov 24, 2020

Description

Per follow-up discussion on #70887 - this pins the version of flutter/plugins to test against to make sure tests for a given commit are deterministic.

Related Issues

#69309

@flutter-dashboard flutter-dashboard bot added the c: contributor-productivity Team-specific productivity, code health, technical debt. label Nov 24, 2020
@google-cla google-cla bot added the cla: yes label Nov 24, 2020
Copy link
Member

@jmagman jmagman left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

Copy link
Contributor

@christopherfujino christopherfujino left a comment

Choose a reason for hiding this comment

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

LGTM. This is really great, thanks!

@christopherfujino
Copy link
Contributor

CI is failing with:

  Failed to load "test/test_test.dart": Invalid argument(s): Uri data:application/dart;charset=utf-8,%20%20%20%20//%20@dart=2.2%0A%20%20%20%20import%20%22dart:isolate%22;%0A%0A%20%20%20%20import%20%22package:test_core/src/bootstrap/vm.dart%22;%0A%0A%20%20%20%20import%20%22file:///b/s/w/ir/k/flutter/dev/bots/test/test_test.dart%22%20as%20test;%0A%0A%20%20%20%20void%20main(_,%20SendPort%20sendPort)%20%7B%0A%20%20%20%20%20%20internalBootstrapVmTest(()%20=%3E%20test.main,%20sendPort);%0A%20%20%20%20%7D%0A%20%20 must have scheme 'file:'.
  package:path/src/style/posix.dart 57:5   PosixStyle.pathFromUri
  package:path/src/context.dart 1000:32    Context.fromUri
  package:path/path.dart 417:32            fromUri
  test.dart 35:67                          flutterRoot
  test.dart                                flutterRoot
  test.dart 42:52                          flutterPluginsVersionFile
  test.dart                                flutterPluginsVersionFile
  test/test_test.dart 64:62                main.<fn>
  package:test_api                         Declarer.group
  package:test_core/test_core.dart 221:13  group
  test/test_test.dart 62:3                 main

I'll take a look...

@christopherfujino
Copy link
Contributor

CI is failing with:

  Failed to load "test/test_test.dart": Invalid argument(s): Uri data:application/dart;charset=utf-8,%20%20%20%20//%20@dart=2.2%0A%20%20%20%20import%20%22dart:isolate%22;%0A%0A%20%20%20%20import%20%22package:test_core/src/bootstrap/vm.dart%22;%0A%0A%20%20%20%20import%20%22file:///b/s/w/ir/k/flutter/dev/bots/test/test_test.dart%22%20as%20test;%0A%0A%20%20%20%20void%20main(_,%20SendPort%20sendPort)%20%7B%0A%20%20%20%20%20%20internalBootstrapVmTest(()%20=%3E%20test.main,%20sendPort);%0A%20%20%20%20%7D%0A%20%20 must have scheme 'file:'.
  package:path/src/style/posix.dart 57:5   PosixStyle.pathFromUri
  package:path/src/context.dart 1000:32    Context.fromUri
  package:path/path.dart 417:32            fromUri
  test.dart 35:67                          flutterRoot
  test.dart                                flutterRoot
  test.dart 42:52                          flutterPluginsVersionFile
  test.dart                                flutterPluginsVersionFile
  test/test_test.dart 64:62                main.<fn>
  package:test_api                         Declarer.group
  package:test_core/test_core.dart 221:13  group
  test/test_test.dart 62:3                 main

I'll take a look...

I think the problem is the line:

final String flutterRoot = path.dirname(path.dirname(path.dirname(path.fromUri(Platform.script)))); not playing well with the memory file system

@amirh
Copy link
Contributor Author

amirh commented Nov 24, 2020

Thanks, looks like Platform.script returns a data URI when executed with pub run test 😕 , looking for a testing friendly alternative

@amirh
Copy link
Contributor Author

amirh commented Nov 24, 2020

I don't see an easy way around the data:uri path when executed with pub run test, ended up allowing the test to override the path to the versions file so that flutterRoot is not evaluated in test. Sadly it means that the test now covers one less thing (the file path).

@christopherfujino
Copy link
Contributor

I don't see an easy way around the data:uri path when executed with pub run test, ended up allowing the test to override the path to the versions file so that flutterRoot is not evaluated in test. Sadly it means that the test now covers one less thing (the file path).

Yeah, I think we hack around this in the flutter tool by checking the scheme, and then doing a regex in tests. Which isn't ideal.

@amirh amirh force-pushed the pin_plugins_version branch from 5532e08 to c06617f Compare November 24, 2020 22:55
@fluttergithubbot fluttergithubbot merged commit 2ceb371 into flutter:master Nov 25, 2020
@amirh amirh deleted the pin_plugins_version branch November 25, 2020 04:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c: contributor-productivity Team-specific productivity, code health, technical debt.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants