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

Null safety migration #155

Merged
merged 23 commits into from
Jan 23, 2024
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
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
18 changes: 1 addition & 17 deletions .github/workflows/dart_ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ jobs:
strategy:
fail-fast: false
matrix:
sdk: [ 2.13.4, stable ]
sdk: [ 2.18.7, 2.19.6 ]
steps:
- uses: actions/checkout@v2
- uses: dart-lang/setup-dart@v0.2
Expand All @@ -71,14 +71,6 @@ jobs:
name: ddc-test-results@${{ matrix.sdk }}
path: reports/${{ matrix.sdk }}/ddc/test-results.json

- name: Report Unit Test Results
uses: dorny/test-reporter@v1
if: ${{ always() && steps.install.outcome == 'success' }}
with:
name: Unit Test Results (ddc - ${{ matrix.sdk }})
path: 'reports/${{ matrix.sdk }}/ddc/test-results.json'
reporter: dart-json

test_dart2js:
permissions:
id-token: write
Expand Down Expand Up @@ -112,11 +104,3 @@ jobs:
with:
name: dart2js-test-results@${{ matrix.sdk }}
path: reports/${{ matrix.sdk }}/dart2js/test-results.json

- name: Report Unit Test Results
uses: dorny/test-reporter@v1
if: ${{ always() && steps.install.outcome == 'success' }}
with:
name: Unit Test Results (dart2js - ${{ matrix.sdk }})
path: 'reports/${{ matrix.sdk }}/dart2js/test-results.json'
reporter: dart-json
61 changes: 30 additions & 31 deletions lib/src/over_react_test/common_component_util.dart
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,17 @@ import 'dart:html';
import 'dart:js';

import 'package:meta/meta.dart';
import 'package:over_react/over_react.dart';
import 'package:over_react/component_base.dart' as component_base;
import 'package:over_react/over_react.dart';
import 'package:over_react_test/over_react_test.dart';
import 'package:over_react_test/src/over_react_test/props_meta.dart';
import 'package:over_react_test/src/over_react_test/test_helpers.dart';
import 'package:react/react_client.dart';
import 'package:react/react_client/js_backed_map.dart';
import 'package:react/react_client/react_interop.dart';
import 'package:react/react_test_utils.dart' as react_test_utils;
import 'package:test/test.dart';

import './custom_matchers.dart';
import './react_util.dart';
import 'dart_util.dart';

/// Run common component tests around default props, prop forwarding, class name merging, and class name overrides.
Expand Down Expand Up @@ -81,16 +80,16 @@ void commonComponentTests(BuilderOnlyUiFactory factory, {
bool shouldTestPropForwarding = true,
List unconsumedPropKeys = const [],
List skippedPropKeys = const [],
List Function(PropsMetaCollection) getUnconsumedPropKeys,
List Function(PropsMetaCollection) getSkippedPropKeys,
List Function(PropsMetaCollection)? getUnconsumedPropKeys,
List Function(PropsMetaCollection)? getSkippedPropKeys,
Map nonDefaultForwardingTestProps = const {},
bool shouldTestClassNameMerging = true,
bool shouldTestClassNameOverrides = true,
bool ignoreDomProps = true,
bool shouldTestRequiredProps = true,
@Deprecated('This flag is not needed as the test will auto detect the version')
bool isComponent2,
dynamic childrenFactory()
bool? isComponent2,
dynamic childrenFactory()?
}) {
childrenFactory ??= _defaultChildrenFactory;

Expand Down Expand Up @@ -132,14 +131,14 @@ Iterable _flatten(Iterable iterable) =>
/// });
/// }
void expectCleanTestSurfaceAtEnd() {
Set<Element> nodesBefore;
late Set<Element> nodesBefore;

setUpAll(() {
nodesBefore = document.body.children.toSet();
nodesBefore = document.body!.children.toSet();
Copy link
Contributor

Choose a reason for hiding this comment

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

i understand that sure a document might not have a body but given how often that is a case this feels ridiculous lol

Copy link
Contributor Author

Choose a reason for hiding this comment

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

lol true

});

tearDownAll(() {
Set<Element> nodesAfter = document.body.children.toSet();
Set<Element> nodesAfter = document.body!.children.toSet();
var nodesAdded = nodesAfter.difference(nodesBefore).map((element) => element.outerHtml).toList();

expect(nodesAdded, isEmpty, reason: 'tests should leave the test surface clean.');
Expand All @@ -161,12 +160,12 @@ void expectCleanTestSurfaceAtEnd() {
/// todo make this public again if there's a need to expose it, once the API has stabilized
@isTest
void _testPropForwarding(BuilderOnlyUiFactory factory, dynamic childrenFactory(), {
@required List unconsumedPropKeys,
@required List skippedPropKeys,
@required List Function(PropsMetaCollection) getUnconsumedPropKeys,
@required List Function(PropsMetaCollection) getSkippedPropKeys,
@required bool ignoreDomProps,
@required Map nonDefaultForwardingTestProps,
required List unconsumedPropKeys,
required List skippedPropKeys,
required List Function(PropsMetaCollection)? getUnconsumedPropKeys,
required List Function(PropsMetaCollection)? getSkippedPropKeys,
required bool ignoreDomProps,
required Map nonDefaultForwardingTestProps,
}) {
testFunction('forwards unconsumed props as expected', () {
// This needs to be retrieved inside a `test`/`setUp`/etc, not inside a group,
Expand Down Expand Up @@ -335,13 +334,13 @@ void _testPropForwarding(BuilderOnlyUiFactory factory, dynamic childrenFactory()
/// Test for prop keys that both are forwarded and exist on the forwarding target's default props.
if (isDartComponent(forwardingTarget)) {
final forwardingTargetType = (forwardingTarget as ReactElement).type as ReactClass;
Map forwardingTargetDefaults;
late Map forwardingTargetDefaults;
switch (forwardingTargetType.dartComponentVersion) { // ignore: invalid_use_of_protected_member
case ReactDartComponentVersion.component: // ignore: invalid_use_of_protected_member
forwardingTargetDefaults = forwardingTargetType.dartDefaultProps; // ignore: deprecated_member_use
forwardingTargetDefaults = forwardingTargetType.dartDefaultProps ?? {}; // ignore: deprecated_member_use
sydneyjodon-wk marked this conversation as resolved.
Show resolved Hide resolved
break;
case ReactDartComponentVersion.component2: // ignore: invalid_use_of_protected_member
forwardingTargetDefaults = JsBackedMap.backedBy(forwardingTargetType.defaultProps);
forwardingTargetDefaults = JsBackedMap.backedBy(forwardingTargetType.defaultProps ?? JsMap());
sydneyjodon-wk marked this conversation as resolved.
Show resolved Hide resolved
break;
}

Expand Down Expand Up @@ -427,7 +426,7 @@ void testClassNameMerging(BuilderOnlyUiFactory factory, dynamic childrenFactory(
..classNameBlacklist = 'blacklisted-class-1 blacklisted-class-2';

var renderedInstance = render(builder(childrenFactory()));
Iterable<Element> forwardingTargetNodes = getForwardingTargets(renderedInstance).map(findDomNode);
final forwardingTargetNodes = getForwardingTargets(renderedInstance).map(findDomNode);

expect(forwardingTargetNodes, everyElement(
allOf(
Expand Down Expand Up @@ -461,7 +460,7 @@ void testClassNameMerging(BuilderOnlyUiFactory factory, dynamic childrenFactory(
/// > Related: [testClassNameMerging]
@isTestGroup
void testClassNameOverrides(BuilderOnlyUiFactory factory, dynamic childrenFactory()) {
Set<String> classesToOverride;
late Set<String> classesToOverride;
var error;

setUp(() {
Expand All @@ -477,7 +476,7 @@ void testClassNameOverrides(BuilderOnlyUiFactory factory, dynamic childrenFactor
// but still fail the test if something goes wrong.
try {
classesToOverride = getForwardingTargets(reactInstanceWithoutOverrides)
.map((target) => findDomNode(target).classes)
.map((target) => findDomNode(target)!.classes)
.expand((CssClassSet classSet) => classSet)
.toSet();
} catch(e) {
Expand All @@ -500,7 +499,7 @@ void testClassNameOverrides(BuilderOnlyUiFactory factory, dynamic childrenFactor
)(childrenFactory())
);

Iterable<Element> forwardingTargetNodes = getForwardingTargets(reactInstance).map(findDomNode);
final forwardingTargetNodes = getForwardingTargets(reactInstance).map(findDomNode);
expect(forwardingTargetNodes, everyElement(
hasExactClasses('')
), reason: '$classesToOverride not overridden');
Expand All @@ -514,7 +513,7 @@ void testClassNameOverrides(BuilderOnlyUiFactory factory, dynamic childrenFactor
/// __Note__: All required props must be provided by [factory].
@isTestGroup
void testRequiredProps(BuilderOnlyUiFactory factory, dynamic childrenFactory()) {
bool isComponent2;
late bool isComponent2;

var keyToErrorMessage = {};
var nullableProps = <String>[];
Expand All @@ -531,7 +530,7 @@ void testRequiredProps(BuilderOnlyUiFactory factory, dynamic childrenFactory())
isComponent2 = version == ReactDartComponentVersion.component2;

var jacket = mount(factory()(childrenFactory()), autoTearDown: false);
var consumedProps = (jacket.getDartInstance() as component_base.UiComponent).consumedProps;
var consumedProps = (jacket.getDartInstance() as component_base.UiComponent).consumedProps ?? [];
sydneyjodon-wk marked this conversation as resolved.
Show resolved Hide resolved
jacket.unmount();

for (var consumedProp in consumedProps) {
Expand All @@ -542,7 +541,7 @@ void testRequiredProps(BuilderOnlyUiFactory factory, dynamic childrenFactory())
requiredProps.add(prop.key);
}

keyToErrorMessage[prop.key] = prop.errorMessage ?? '';
keyToErrorMessage[prop.key] = prop.errorMessage;
}
}
});
Expand Down Expand Up @@ -580,8 +579,8 @@ void testRequiredProps(BuilderOnlyUiFactory factory, dynamic childrenFactory())
void component2RequiredPropsTest() {
PropTypes.resetWarningCache();

List<String> consoleErrors = [];
JsFunction originalConsoleError = context['console']['error'];
var consoleErrors = <String?>[];
final originalConsoleError = context['console']['error'] as JsFunction;
addTearDown(() => context['console']['error'] = originalConsoleError);
context['console']['error'] = JsFunction.withThis((self, [message, arg1, arg2, arg3, arg4, arg5]) {
consoleErrors.add(message);
Expand All @@ -590,7 +589,7 @@ void testRequiredProps(BuilderOnlyUiFactory factory, dynamic childrenFactory())
});

final reactComponentFactory = factory().componentFactory as
ReactDartComponentFactoryProxy2; // ignore: avoid_as
ReactDartComponentFactoryProxy2; // ignore: avoid_as

for (var propKey in requiredProps) {
if (!reactComponentFactory.defaultProps.containsKey(propKey)) {
Expand Down Expand Up @@ -657,8 +656,8 @@ void testRequiredProps(BuilderOnlyUiFactory factory, dynamic childrenFactory())
} else {
PropTypes.resetWarningCache();

List<String> consoleErrors = [];
JsFunction originalConsoleError = context['console']['error'];
var consoleErrors = <String?>[];
final originalConsoleError = context['console']['error'] as JsFunction;
addTearDown(() => context['console']['error'] = originalConsoleError);
context['console']['error'] = JsFunction.withThis((self, [message, arg1, arg2, arg3, arg4, arg5]) {
consoleErrors.add(message);
Expand Down
12 changes: 6 additions & 6 deletions lib/src/over_react_test/console_log_utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,12 @@ import 'package:react/react_client/react_interop.dart';
/// props that are not valid, the errors will be caught to allow the test to complete.
///
/// To handle asynchronous behavior, see [recordConsoleLogsAsync].
List<String> recordConsoleLogs(
List<String?> recordConsoleLogs(
Function() callback, {
ConsoleConfiguration configuration = allConfig,
bool shouldResetPropTypesWarningCache = true,
}) {
final consoleLogs = <String>[];
final consoleLogs = <String?>[];
final logTypeToCapture = configuration.logType == 'all'
? ConsoleConfiguration.types
: [configuration.logType];
Expand All @@ -54,7 +54,7 @@ List<String> recordConsoleLogs(
// NOTE: Using console.log or print within this function will cause an infinite
// loop when the logType is set to `log`.
consoleLogs.add(message);
consoleRefs[config]
consoleRefs[config]!
.apply([message, arg1, arg2, arg3, arg4, arg5], thisArg: self);
});
}
Expand All @@ -81,12 +81,12 @@ List<String> recordConsoleLogs(
/// with the exception being the provided callback should be asynchronous.
///
/// Related: [recordConsoleLogs]
FutureOr<List<String>> recordConsoleLogsAsync(
FutureOr<List<String?>> recordConsoleLogsAsync(
Future Function() asyncCallback, {
ConsoleConfiguration configuration = allConfig,
bool shouldResetPropTypesWarningCache = true,
}) async {
var consoleLogs = <String>[];
var consoleLogs = <String?>[];
final logTypeToCapture = configuration.logType == 'all'
? ConsoleConfiguration.types
: [configuration.logType];
Expand All @@ -101,7 +101,7 @@ FutureOr<List<String>> recordConsoleLogsAsync(
// NOTE: Using console.log or print within this function will cause an infinite
// loop when the logType is set to `log`.
consoleLogs.add(message);
consoleRefs[config]
consoleRefs[config]!
.apply([message, arg1, arg2, arg3, arg4, arg5], thisArg: self);
});
}
Expand Down
28 changes: 12 additions & 16 deletions lib/src/over_react_test/custom_matchers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
import 'dart:html';
import 'dart:svg';

import 'package:collection/collection.dart' show IterableNullableExtension;
import 'package:over_react/over_react.dart';
import 'package:matcher/matcher.dart';
import 'package:over_react_test/src/over_react_test/dart_util.dart';
import 'package:react/react.dart' as react;
import 'package:react/react_test_utils.dart' as react_test_utils;
Expand Down Expand Up @@ -46,7 +46,7 @@ class ClassNameMatcher extends Matcher {
static Iterable getClassIterable(dynamic classNames) {
Iterable classes;
if (classNames is Iterable<String>) {
classes = classNames.where((className) => className != null).expand(splitSpaceDelimitedString);
classes = classNames.whereNotNull().expand(splitSpaceDelimitedString);
} else if (classNames is String) {
classes = splitSpaceDelimitedString(classNames);
} else {
Expand All @@ -61,7 +61,7 @@ class ClassNameMatcher extends Matcher {
// There's a bug in DDC where, though the docs say `className` should
// return `String`, it will return `AnimatedString` for `SvgElement`s. See
// https://github.com/dart-lang/sdk/issues/36200.
String castClassName;
String? castClassName;
if (className is String) {
castClassName = className;
} else if (className is AnimatedString) {
Expand Down Expand Up @@ -164,7 +164,7 @@ class _HasToStringValue extends CustomMatcher {
_HasToStringValue(matcher) : super('Object with toString() value', 'toString()', matcher);

@override
featureValueOf(Object item) => item.toString();
featureValueOf(Object? item) => item.toString();
}

class _HasPropMatcher extends CustomMatcher {
Expand All @@ -187,7 +187,7 @@ class _HasPropMatcher extends CustomMatcher {

@override
Map featureValueOf(item) {
if (_useDomAttributes(item)) return findDomNode(item).attributes;
if (_useDomAttributes(item)) return findDomNode(item)!.attributes;
if (item is react.Component) return item.props;

return getProps(item);
Expand Down Expand Up @@ -269,7 +269,7 @@ class _IsFocused extends Matcher {
..add('is not a valid Element.');
}

if (!document.documentElement.contains(item)) {
if (!document.documentElement!.contains(item)) {
return mismatchDescription
..add('is not attached to the document, and thus cannot be focused.')
..add(' If testing with React, you can use `renderAttachedToDocument`.');
Expand Down Expand Up @@ -314,7 +314,7 @@ Matcher throwsPropError(String propName, [String message = '']) {
///
/// __Note__: The [message] is matched rather than the [Error] instance due to Dart's wrapping of all `throw`
/// as a [DomException]
Matcher throwsPropError_Required(String propName, [String message = '']) {
Matcher throwsPropError_Required(String propName, [String? message = '']) {
return throwsA(anyOf(
hasToStringValue('V8 Exception'), /* workaround for https://github.com/dart-lang/sdk/issues/26093 */
hasToStringValue(contains('RequiredPropError: Prop $propName is required. $message'.trim()))
Expand Down Expand Up @@ -362,10 +362,10 @@ Matcher throwsPropError_Combination(String propName, String prop2Name, [String m
/// and pass it to [recordConsoleLogs] to run the function and record the resulting
/// logs that are emitted during the function runtime.
class _LoggingFunctionMatcher extends CustomMatcher {
_LoggingFunctionMatcher(dynamic matcher, {this.config, String description, String name, bool onlyIfAssertsAreEnabled = false})
_LoggingFunctionMatcher(dynamic matcher, {this.config, String? description, String? name, bool onlyIfAssertsAreEnabled = false})
: super(description ?? 'emits the logs', name ?? 'logs', _wrapMatcherForSingleLog(matcher, onlyIfAssertsAreEnabled));

final ConsoleConfiguration config;
final ConsoleConfiguration? config;

static dynamic _wrapMatcherForSingleLog(dynamic expected, [bool onlyIfAssertsAreEnabled = false]) {
if (onlyIfAssertsAreEnabled && !assertsEnabled()) return anything;
Expand All @@ -375,17 +375,13 @@ class _LoggingFunctionMatcher extends CustomMatcher {

@override
featureValueOf(actual) {
var logs = <String>[];

if (actual is List) return actual;

if (actual is! Function()) {
throw ArgumentError('The actual value must be a callback or a List.');
}

logs = recordConsoleLogs(actual, configuration: config ?? logConfig);

return logs;
return recordConsoleLogs(actual, configuration: config ?? logConfig);
}
}

Expand All @@ -400,7 +396,7 @@ class _LoggingFunctionMatcher extends CustomMatcher {
/// caught error.
///
/// Related: [logsToConsole], [hasNoLogs]
Matcher hasLog(dynamic expected, {ConsoleConfiguration consoleConfig, bool onlyIfAssertsAreEnabled = false}) =>
Matcher hasLog(dynamic expected, {ConsoleConfiguration? consoleConfig, bool onlyIfAssertsAreEnabled = false}) =>
_LoggingFunctionMatcher(anyElement(contains(expected)), config: consoleConfig, onlyIfAssertsAreEnabled: onlyIfAssertsAreEnabled);

/// A Matcher used to compare a list of logs against a provided matcher.
Expand Down Expand Up @@ -440,7 +436,7 @@ Matcher hasLog(dynamic expected, {ConsoleConfiguration consoleConfig, bool onlyI
/// ```
///
/// Related: [hasLog], [hasNoLogs]
Matcher logsToConsole(dynamic expected, {ConsoleConfiguration consoleConfig, bool onlyIfAssertsAreEnabled = false}) =>
Matcher logsToConsole(dynamic expected, {ConsoleConfiguration? consoleConfig, bool onlyIfAssertsAreEnabled = false}) =>
_LoggingFunctionMatcher(expected, config: consoleConfig, onlyIfAssertsAreEnabled: onlyIfAssertsAreEnabled);

/// A matcher to verify that a callback function does not emit any logs.
Expand Down
Loading
Loading