Skip to content
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.

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

FED-734 CI cleanup: remove failing stable config, add 2.18 config #805

Merged
merged 16 commits into from
Feb 8, 2023
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 10 additions & 52 deletions .github/workflows/dart_ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@ jobs:
fail-fast: false
matrix:
# Can't run on `dev` (Dart 3) until we're fully null-safe.
# TODO re-enable stable once we get the analysis/test parts of the build passing
sdk: [ 2.13.4 ]
# We can't run on Dart >=2.19 until https://github.com/dart-lang/sdk/issues/51128
# is fixed, or we work around it, which requires upgrading to analyzer ^5.1.0.
sdk: [ 2.13.4, 2.18.7 ]
steps:
- uses: actions/checkout@v2
- uses: dart-lang/setup-dart@v0.2
Expand All @@ -44,21 +45,21 @@ jobs:
run: |
# Analyze lib to ensure public APIs don't depend on build-to-cache files,
# which could cause analysis issues for consumers who haven't run a build yet.
dartanalyzer lib
cd example/boilerplate_versions
dart analyze
dart analyze lib
dart analyze example/boilerplate_versions
if: always() && steps.install.outcome == 'success'

- id: build
timeout-minutes: 6
name: Build generated files / precompile DDC assets
run: |
dart pub run build_runner build --delete-conflicting-outputs -o ddc_precompiled
if [ ${{ matrix.sdk }} = '2.7.2' ]; then
if [ ${{ matrix.sdk }} = '2.18.7' ]; then
git diff --exit-code
else
# Exclude built_redux generated files since they get generated differently in Dart 2.7 vs other versions
git diff --exit-code -- ":(exclude)test/over_react/component_declaration/redux_component_test/test_reducer.g.dart"
# Don't check these generated files for other SDKs, since they may generate differently
# due to different resolved dependencies.
git diff --exit-code -- ":(exclude)test/mockito.mocks.dart" ":(exclude)test/over_react/component_declaration/redux_component_test/test_reducer.g.dart"
fi
if: always() && steps.install.outcome == 'success'

Expand All @@ -79,49 +80,6 @@ jobs:
run: dart pub run dart_dev test --build-args="-r" -P dart2js
if: always() && steps.install.outcome == 'success' && steps.build.outcome == 'success'

validate_analyzer:
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
sdk: [ 2.13.4, stable ]
analyzer:
- ^0.40.0
- ^0.41.0
# No ^0.42.0 since that line only contains 0.42.0-nullsafety.0, which is the 1.0.0 prerelease
- ^1.0.0
# For stable, only validate analyzer ^1.0.0. Otherwise, validate all analyzer versions.
exclude:
- sdk: stable
include:
- sdk: stable
analyzer: ^1.0.0
steps:
- uses: actions/checkout@v2
- uses: dart-lang/setup-dart@v0.2
with:
sdk: ${{ matrix.sdk }}

- name: Print Dart SDK version
run: dart --version

- name: Update analyzer constraint to ${{ matrix.analyzer }} and validate `pub get` can resolve
id: resolve
run: |
dart tool/set_analyzer_constraint.dart "${{ matrix.analyzer }}"
# Show the updated version constraint
git diff pubspec.yaml
dart pub get

- name: Analyze package source
run: dart analyze .

- name: Verify builder runs without errors
run: dart run build_runner build --build-filter='**.dart' --delete-conflicting-outputs

- name: Run builder tests
run: dart run test -p vm -- test/vm_tests/builder

analyzer_plugin:
runs-on: ubuntu-latest
defaults:
Expand All @@ -131,7 +89,7 @@ jobs:
fail-fast: false
matrix:
# Can't run on `dev` (Dart 3) until we're fully null-safe.
sdk: [ 2.13.4, stable ]
sdk: [ 2.13.4, 2.18.7, stable ]
steps:
- uses: actions/checkout@v2
- uses: dart-lang/setup-dart@v0.2
Expand Down
2 changes: 1 addition & 1 deletion app/over_react_redux/todo_client/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ environment:
dependencies:
color: any
memoize: ^2.0.0
meta: ^1.6.0 # Workaround to avoid https://github.com/dart-lang/sdk/issues/46142
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These comments were outdated, so I cleaned them up

meta: ^1.6.0
over_react: ">=3.1.5 <5.0.0"
json_annotation: ^3.0.0
redux: ">=3.0.0 <5.0.0"
Expand Down
7 changes: 3 additions & 4 deletions build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,10 @@ builders:
targets:
$default:
builders:
# mockito's builder is expensive and is not needed until this package is
# migrated to null-safety. At that point, it should be scoped only to
# relevant files.
mockito:mockBuilder:
enabled: false
generate_for:
include:
- "test/mockito.dart"
over_react|_over_react_local_builder:
enabled: true
generate_for:
Expand Down
4 changes: 2 additions & 2 deletions pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ dependencies:
dart_style: '>=1.2.5 <3.0.0'
js: ^0.6.1+1
logging: '>=0.11.3+2 <2.0.0'
meta: ^1.6.0 # Workaround to avoid https://github.com/dart-lang/sdk/issues/46142
meta: ^1.6.0
path: ^1.5.1
react: ^6.1.8
redux: ">=3.0.0 <5.0.0"
Expand All @@ -36,7 +36,7 @@ dev_dependencies:
dependency_validator: '>=2.0.0 <4.0.0'
glob: '>=1.2.0<3.0.0'
io: '>=0.3.2+1 <2.0.0'
mockito: ^4.1.1
mockito: ^5.0.0
react_testing_library: ^2.1.0
over_react_test: ^2.10.2
pedantic: ^1.8.0
Expand Down
25 changes: 25 additions & 0 deletions test/mockito.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import 'package:logging/logging.dart';
import 'package:mockito/annotations.dart';

@GenerateMocks([], customMocks: [
MockSpec<Logger>(returnNullOnMissingStub: true),
MockSpec<List>(fallbackGenerators: {
#[]: listIndexOperatorShim,
#removeAt: listRemoveAtShim,
#removeLast: listRemoveLastShim,
}, returnNullOnMissingStub: true),
MockSpec<Map>(fallbackGenerators: {
#update: mapUpdateShim,
#putIfAbsent: mapPutIfAbsentShim,
}, returnNullOnMissingStub: true)
])
void main() {}

dynamic listIndexOperatorShim(int index) => 1;
dynamic listRemoveAtShim(int index) => 1;
dynamic listRemoveLastShim() => 1;

String mapUpdateShim<K, V>(K key, V Function(V value) update,
{V Function() ifAbsent}) =>
'value';
String mapPutIfAbsentShim<K, V>(K key, V Function() ifAbsent) => 'value';
30 changes: 30 additions & 0 deletions test/mockito.mocks.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Mocks generated by Mockito 5.0.15 from annotations
// in over_react/test/mockito.dart.
// Do not manually edit this file.

// ignore_for_file: no_leading_underscores_for_library_prefixes
import 'package:logging/src/logger.dart' as _i2;
import 'package:mockito/mockito.dart' as _i1;

// 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

/// A class which mocks [Logger].
///
/// See the documentation for Mockito's code generation for more information.
class MockLogger extends _i1.Mock implements _i2.Logger {}

/// A class which mocks [List].
///
/// See the documentation for Mockito's code generation for more information.
class MockList<E> extends _i1.Mock implements List<E> {}

/// A class which mocks [Map].
///
/// See the documentation for Mockito's code generation for more information.
class MockMap<K, V> extends _i1.Mock implements Map<K, V> {}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions test/over_react/shared/map_proxy_tests.dart
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ library over_react_tests.shared.map_proxy_tests;
import 'package:mockito/mockito.dart';
import 'package:test/test.dart';

import '../../mockito.mocks.dart';

void mapProxyTests(Map Function(Map proxiedMap) mapProxyFactory) {
group('proxies the Map member:', () {
Map proxy;
Expand Down Expand Up @@ -173,5 +175,3 @@ void mapProxyTests(Map Function(Map proxiedMap) mapProxyFactory) {
});
});
}

class MockMap extends Mock implements Map {}
2 changes: 1 addition & 1 deletion test/over_react/util/prop_conversion_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1072,7 +1072,7 @@ main() {
isA<Object>().havingToStringValue(anyOf(
// DDC error message
matches(RegExp(
r"Expected a value of type 'Map[^']*', but got one of type 'NativeJavaScriptObject'")),
r"Expected a value of type 'Map[^']*', but got one of type '(Native|Legacy)JavaScriptObject'")),
// dart2js error message
matches(RegExp(
r"type '(Unknown|Plain)JavaScriptObject' is not a subtype of type 'Map[^']*'")),
Expand Down
24 changes: 15 additions & 9 deletions test/over_react/util/react_wrappers_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1063,19 +1063,25 @@ main() {
expect(result2, same(result1), reason: 'should have returned the same object');
}, tags: 'no-ddc');

test('unless the runtime is the DDC', () {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Basically, before we were testing that caching wasn't happening in DDC, which wasn't really the behavior we wanted ensure. I replaced it with a test that validates that it doesn't break, which is really the behavior we're after, at least for older SDKs.

// A previous version of this test asserted that caching didn't take place, but that's
// no longer true in newer SDKs since Expando started using WeakMap in Dart 2.14.0: https://github.com/dart-lang/sdk/commit/460887d8149748d3feaad857f1b13721faadeffa,
// so we can't test that behavior having different tests for different SDKs.
// Once we raise our minimums, we can update the test above to run on DDC as well.
// Until then, just ensure the implementation isn't broken in DDC, as a regression test for older SDKs.
test('and works without throwing in DDC', () {
final element = factory({
'dartProp': 'dart'
}) as ReactElement;

var result1 = getProps(element);
var result2 = getProps(element);

expect(result1, containsPair('dartProp', 'dart'), reason: 'test setup sanity check');
expect(result2, isNot(same(result1)),
reason: 'if this test fails, then it\'s possible that the bug was fixed in'
' a newer version of the Dart SDK, and this test can be removed!');
}, tags: 'ddc');
dynamic result1;
dynamic result2;
expect(() {
result1 = getProps(element);
result2 = getProps(element);
}, returnsNormally);
expect(result1, containsPair('dartProp', 'dart'));
expect(result2, containsPair('dartProp', 'dart'));
});
});
}

Expand Down
5 changes: 2 additions & 3 deletions test/over_react_redux/value_mutation_checker_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,13 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import 'package:mockito/mockito.dart';
import 'package:over_react/over_react.dart';
import 'package:over_react/src/over_react_redux/value_mutation_checker.dart';
import 'package:react/react.dart' as react;
import 'package:test/test.dart';

import '../mockito.mocks.dart';

// ignore_for_file: invalid_use_of_protected_member
main() {
group('Value Mutation Checker:', () {
Expand Down Expand Up @@ -150,5 +151,3 @@ void sharedHashTests(InstanceHasher Function() getHasher) {
});
});
}

class MockList extends Mock implements List {}
67 changes: 0 additions & 67 deletions test/test_util/mock_classes.dart

This file was deleted.

1 change: 1 addition & 0 deletions test/vm_tests/builder/codegen_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import 'package:over_react/src/builder/codegen.dart';
import 'package:source_span/source_span.dart';
import 'package:test/test.dart';

import '../../mockito.mocks.dart';
import './util.dart';

main() {
Expand Down
1 change: 1 addition & 0 deletions test/vm_tests/builder/declaration_parsing_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import 'package:over_react/src/component_declaration/annotations.dart' as annota
import 'package:source_span/source_span.dart';
import 'package:test/test.dart';

import '../../mockito.mocks.dart';
import './util.dart';
import 'parsing/parsing_helpers.dart';

Expand Down
2 changes: 1 addition & 1 deletion test/vm_tests/builder/parsing/error_collection.dart
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import 'package:over_react/src/builder/parsing.dart';
import 'package:source_span/source_span.dart';
import 'package:test/test.dart';

import '../util.dart';
import '../../../mockito.mocks.dart';

main() {
group('error collection -', () {
Expand Down
Loading