Generator output is non deterministic if several imports could be used to get the same symbol

tienlocbui1110 commented 2 years ago

Description

I need to create an automation task that detect devs have called build_runner on their client.
Problem
Despite nothing has changed, sometimes CI build failed because it generates new contents.

Let's take a look at the code below.

dummy_widget.dart

import 'package:flutter/cupertino.dart';
import 'package:flutter/material.dart';
import 'package:mokito_test/platform.dart';

class DummyWidget extends StatefulWidget {
  final DummyView view;

  DummyWidget({Key? key, DummyView? view})
      : view = view ?? DummyView(),
        super(key: key);

  @override
  _DummyWidgetState createState() => _DummyWidgetState();
}

class _DummyWidgetState extends State<DummyWidget> {
  final FocusNode _focusNode = FocusNode();
  _DummyWidgetState();

  @override
  Widget build(BuildContext context) {
    return Column(
      children: [
        TextFormField(
          focusNode: _focusNode,
        ),
        if (Platform.isAndroid)
          MaterialButton(
            child: const Text('Dummy'),
            onPressed: () {
              widget.view.unFocus(_focusNode);
            },
          ),
        if (Platform.isIOS)
          CupertinoButton(
            child: const Text('Dummy'),
            onPressed: () {
              widget.view.unFocus(_focusNode);
            },
          ),
      ],
    );
  }
}

class DummyView {
  void unFocus(FocusNode? focusNode) {
    focusNode?.unfocus();
  }
}

dummy_test.dart

import 'package:mockito/annotations.dart';
import 'package:mokito_test/dummy_widget.dart';

@GenerateMocks([], customMocks: [
  MockSpec<DummyView>(
    as: #MockDummyView,
    returnNullOnMissingStub: true,
  )
])
void main() {
  // Some test
}

After generate multiple times, I have two output here:

dummy_test.mocks.dart

// Mocks generated by Mockito 5.1.0 from annotations
// in mokito_test/test/dummy_test.dart.
// Do not manually edit this file.

import 'package:flutter/material.dart' as _i3;
import 'package:mockito/mockito.dart' as _i1;
import 'package:mokito_test/dummy_widget.dart' as _i2;

// ignore_for_file: type=lint
// ignore_for_file: avoid_redundant_argument_values
// ignore_for_file: avoid_setters_without_getters
// ignore_for_file: comment_references
// ignore_for_file: implementation_imports
// ignore_for_file: invalid_use_of_visible_for_testing_member
// ignore_for_file: prefer_const_constructors
// ignore_for_file: unnecessary_parenthesis
// ignore_for_file: camel_case_types

/// A class which mocks [DummyView].
///
/// See the documentation for Mockito's code generation for more information.
class MockDummyView extends _i1.Mock implements _i2.DummyView {
  @override
  void unFocus(_i3.FocusNode? focusNode) =>
      super.noSuchMethod(Invocation.method(#unFocus, [focusNode]),
          returnValueForMissingStub: null);
}

dummy_test.mocks.dart

// Mocks generated by Mockito 5.1.0 from annotations
// in mokito_test/test/dummy_test.dart.
// Do not manually edit this file.

import 'package:flutter/cupertino.dart' as _i3;
import 'package:mockito/mockito.dart' as _i1;
import 'package:mokito_test/dummy_widget.dart' as _i2;

// ignore_for_file: type=lint
// ignore_for_file: avoid_redundant_argument_values
// ignore_for_file: avoid_setters_without_getters
// ignore_for_file: comment_references
// ignore_for_file: implementation_imports
// ignore_for_file: invalid_use_of_visible_for_testing_member
// ignore_for_file: prefer_const_constructors
// ignore_for_file: unnecessary_parenthesis
// ignore_for_file: camel_case_types

/// A class which mocks [DummyView].
///
/// See the documentation for Mockito's code generation for more information.
class MockDummyView extends _i1.Mock implements _i2.DummyView {
  @override
  void unFocus(_i3.FocusNode? focusNode) =>
      super.noSuchMethod(Invocation.method(#unFocus, [focusNode]),
          returnValueForMissingStub: null);
}

As you can see, the different between two files all about randomly import. FocusNode can be imported from cupertino and material as well, so they randomly change the contents, which I don't want.

I have try to import in dummy_widget.dart like this, but have no effect.

import 'package:flutter/material.dart' as m;
// ....
class DummyView {
  void unFocus(m.FocusNode? focusNode) {
    focusNode?.unfocus();
  }
}

Environment

Dart SDK version: 2.15.0 build_runner: 2.1.5 mockito: 5.1.0

Question

How can I get rid of the random of generated codes to make efficiently automation?

srawlins commented 2 years ago

Great bug report, thanks! Definitely a bug somewhere in mockito, analyzer, build, code_builder, or source_gen...

YazeedAlKhalaf commented 2 years ago

@srawlins can you elaborate more so we can help because we are facing the same issue?

srawlins commented 2 years ago

I don't have a repro and have not investigated, but looking at @tienlocbui1110 's code above, in the first case, FocusNode is imported from 'package:flutter/material.dart', and in the second, 'package:flutter/cupertino.dart'. This is the only difference I see.

YazeedAlKhalaf commented 2 years ago

yes, this is exactly the problem.

for us, in CI we check if the files have been generated or not by running the command and checking the git status. when this ranom thingy happens, the PR fails.

we need the mocks to be the same if the source files haven't changed.

yanok commented 1 year ago

The problem is that both 'package:flutter/material.dart' and 'package:flutter/cupertino.dart' export 'package:flutter/widgets.dart', so the contents of the latter is kinda imported twice, and I guess Mockito's codegen randomly picks one at runtime, assuming there will be only one. We need to sort the possible imports before picking one, that would fix the issue.

yanok commented 1 year ago

I didn't debug it, but I'd try changing https://github.com/dart-lang/mockito/blob/cca4858ad0f1e3930ebdcc6faf4a918616cca48c/lib/src/builder.dart#L151 to use SplayTreeSet instead and see if it helps.

dart-lang / mockito