@christopherfujino I'm not sure how to fix this failure. I attempted to update the failing test to use testUsingContext(...) which required an import of test/src/context.dart, resulting in the following compile time error:

dart run test test/general.shard/flutter_manifest_test.dart 
00:07 +0 -1: loading test/general.shard/flutter_manifest_test.dart [E]                                                                                                                                                                       
  Failed to load "test/general.shard/flutter_manifest_test.dart":
  test/src/context.dart:5:1: Error: A library can't opt out of null safety by default, when using sound null safety.
  // @dart = 2.8
  ^^^^^^^^^^^^^^
  lib/src/context_runner.dart:5:1: Error: A library can't opt out of null safety by default, when using sound null safety.
  // @dart = 2.8
  ^^^^^^^^^^^^^^
  lib/src/devtools_launcher.dart:5:1: Error: A library can't opt out of null safety by default, when using sound null safety.
  // @dart = 2.8
  ^^^^^^^^^^^^^^
  lib/src/resident_runner.dart:5:1: Error: A library can't opt out of null safety by default, when using sound null safety.
  // @dart = 2.8
  ^^^^^^^^^^^^^^
  lib/src/run_hot.dart:5:1: Error: A library can't opt out of null safety by default, when using sound null safety.
  // @dart = 2.8
  ^^^^^^^^^^^^^^
  lib/src/resident_devtools_handler.dart:5:1: Error: A library can't opt out of null safety by default, when using sound null safety.
  // @dart = 2.8
  ^^^^^^^^^^^^^^
  lib/src/run_cold.dart:5:1: Error: A library can't opt out of null safety by default, when using sound null safety.
  // @dart = 2.8
  ^^^^^^^^^^^^^^
00:07 +0 -1: Some tests failed.

It looks like flutter_manifest_test has already been migrated to null safety and that's causing the issues. Can you take a look?

looking

looking

So there are several issues with this, especially surrounding the test setup.

However, I think there may legitimately be a breaking change in package:pub_semver (I don't see a 2.1.0 tag on the github repo, so I'm not able to diff the exact changes in this release):

When I run this test:

  testUsingContext('FlutterManifest parses major.minor+build version clause', () {
    const String manifest = '''
name: test
version: 1.0+2
dependencies:
  flutter:
    sdk: flutter
flutter:
''';
    final BufferLogger logger = BufferLogger.test();
    final FlutterManifest flutterManifest = FlutterManifest.createFromString(
      manifest,
      logger: logger,
    );

    expect(flutterManifest, matchesManifest(
      appVersion: '1.0+2',
      buildName: '1.0',
      buildNumber: '2',
    ));
  });

I get this error:

00:01 +0 -1: FlutterManifest parses major.minor+build version clause [E]
  Expected: <Instance of 'FlutterManifest'> with `appVersion`: '1.0+2' and `buildName`: '1.0' and `buildNumber`: '2'
    Actual: <Instance of 'FlutterManifest'>
     Which: has `appVersion` with value <null> which not an <Instance of 'String'>

When I revert the pub_semver version back to 2.1.0, this test passes.

Oh wait, I think if we just change the sample manifest to have this line:

version: 1.0.0+2

and then update the expectations it will work

However, I think there may legitimately be a breaking change in package:pub_semver

How ironic that we potentially didn't follow semver when releasing a new version of package:pub_semver... :-)

Is this maybe just worth pinning at 2.1.0 for now?

this was the breaking change: dart-archive/pub_semver@cee044a

and yes, very ironic ;)

So, to summarize:

1. this test uses the testWithoutContext function while in the failure mode it actually depends on the context, thus the true test failure was obscured
2. a fix was made to pub_semver that was correct, but probably should have bumped the x
3. the test used an invalid version number. fixing the test to use 1.0.0 should unblock this roll.

So, to summarize:

1. this test uses the testWithoutContext function while in the failure mode it actually depends on the context, thus the true test failure was obscured
2. a fix was made to pub_semver that was correct, but probably should have bumped the x
3. the test used an invalid version number. fixing the test to use 1.0.0 should unblock this roll.

Awesome, thanks for investigating! To confirm I understand correctly, we should be able to resolve the issue by simply updating the sample manifest, right?

So, to summarize:

1. this test uses the testWithoutContext function while in the failure mode it actually depends on the context, thus the true test failure was obscured
2. a fix was made to pub_semver that was correct, but probably should have bumped the x
3. the test used an invalid version number. fixing the test to use 1.0.0 should unblock this roll.

Awesome, thanks for investigating! To confirm I understand correctly, we should be able to resolve the issue by simply updating the sample manifest, right?

Yep, and the expectation. 1.0+2 was never valid semver anyway.

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

• The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.