Merge pull request #98 from Workiva/release_over_react_test_2.9.2

RM-72281 Release over_react_test 2.9.2

Author: rmconsole2-wf

lib/src/over_react_test/common_component_util.dart

@@ -86,11 +86,10 @@ void commonComponentTests(BuilderOnlyUiFactory factory, {
   bool ignoreDomProps = true,
   bool shouldTestRequiredProps = true,
   @Deprecated('This flag is not needed as the test will auto detect the version')
-  bool isComponent2 = false,
+  bool isComponent2,
   dynamic childrenFactory()
 }) {
   childrenFactory ??= _defaultChildrenFactory;
-  isComponent2 = ReactDartComponentVersion.fromType((factory()()).type) == '2' || isComponent2;
 
   if (shouldTestPropForwarding) {
     _testPropForwarding(
@@ -112,7 +111,7 @@ void commonComponentTests(BuilderOnlyUiFactory factory, {
     testClassNameOverrides(factory, childrenFactory);
   }
   if (shouldTestRequiredProps) {
-    testRequiredProps(factory, childrenFactory, isComponent2);
+    testRequiredProps(factory, childrenFactory);
   }
 }
 
@@ -427,13 +426,23 @@ void testClassNameOverrides(BuilderOnlyUiFactory factory, dynamic childrenFactor
 /// > Typically not consumed standalone. Use [commonComponentTests] instead.
 ///
 /// __Note__: All required props must be provided by [factory].
-void testRequiredProps(BuilderOnlyUiFactory factory, dynamic childrenFactory(),
-    bool isComponent2) {
+void testRequiredProps(BuilderOnlyUiFactory factory, dynamic childrenFactory()) {
+  bool isComponent2;
+
   var keyToErrorMessage = {};
   var nullableProps = <String>[];
   var requiredProps = <String>[];
 
   setUp(() {
+    // This can't go in a setUpAll since it would be called before consumer setUps.
+    //
+    // ignore: invalid_use_of_protected_member
+    final version = ReactDartComponentVersion.fromType(
+      (factory()(childrenFactory())).type,
+    );
+    // ignore: invalid_use_of_protected_member
+    isComponent2 = version == ReactDartComponentVersion.component2;
+
     var jacket = mount(factory()(childrenFactory()), autoTearDown: false);
     var consumedProps = (jacket.getDartInstance() as component_base.UiComponent).consumedProps;
     jacket.unmount();
@@ -451,37 +460,37 @@ void testRequiredProps(BuilderOnlyUiFactory factory, dynamic childrenFactory(),
     }
   });
 
-  if (!isComponent2) {
-    test('throws when the required prop is not set or is null', () {
-        for (var propKey in requiredProps) {
-          final reactComponentFactory = factory()
-              .componentFactory as ReactDartComponentFactoryProxy; // ignore: avoid_as
-
-          // Props that are defined in the default props map will never not be set.
-          if (!reactComponentFactory.defaultProps.containsKey(propKey)) {
-            var badRenderer = () =>
-                render((factory()
-                  ..remove(propKey)
-                )(childrenFactory()));
-
-            expect(badRenderer,
-                throwsPropError_Required(propKey, keyToErrorMessage[propKey]),
-                reason: '$propKey is not set');
-          }
+  test('throws (component1) or logs the correct errors (component2) when the required prop is not set or is null', () {
+    void component1RequiredPropsTest(){
+      for (var propKey in requiredProps) {
+        final reactComponentFactory = factory()
+            .componentFactory as ReactDartComponentFactoryProxy; // ignore: avoid_as
 
-          var propsToAdd = {propKey: null};
+        // Props that are defined in the default props map will never not be set.
+        if (!reactComponentFactory.defaultProps.containsKey(propKey)) {
           var badRenderer = () =>
               render((factory()
-                ..addAll(propsToAdd)
+                ..remove(propKey)
               )(childrenFactory()));
 
           expect(badRenderer,
               throwsPropError_Required(propKey, keyToErrorMessage[propKey]),
-              reason: '$propKey is set to null');
+              reason: '$propKey is not set');
         }
-    });
-  } else {
-    test('logs the correct errors when the required prop is not set or is null', () {
+
+        var propsToAdd = {propKey: null};
+        var badRenderer = () =>
+            render((factory()
+              ..addAll(propsToAdd)
+            )(childrenFactory()));
+
+        expect(badRenderer,
+            throwsPropError_Required(propKey, keyToErrorMessage[propKey]),
+            reason: '$propKey is set to null');
+      }
+    }
+
+    void component2RequiredPropsTest() {
       PropTypes.resetWarningCache();
 
       List<String> consoleErrors = [];
@@ -534,8 +543,10 @@ void testRequiredProps(BuilderOnlyUiFactory factory, dynamic childrenFactory(),
         consoleErrors = [];
         PropTypes.resetWarningCache();
       }
-    });
-  }
+    }
+
+    isComponent2 ? component2RequiredPropsTest() : component1RequiredPropsTest();
+  });
 
   test('nullable props', () {
     if (!isComponent2) {

pubspec.yaml

@@ -1,5 +1,5 @@
 name: over_react_test
-version: 2.9.1
+version: 2.9.2
 description: A library for testing OverReact components
 author: Workiva UI Platform Team <[email protected]>
 homepage: https://github.com/Workiva/over_react_test/

test/over_react_test/common_component_util_test.dart

@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+import 'package:over_react/over_react.dart';
 import 'package:over_react_test/src/over_react_test/props_meta.dart';
 import 'package:test/test.dart';
 import 'package:over_react_test/over_react_test.dart';
@@ -80,5 +81,71 @@ main() {
       );
       });
     });
+
+    // This causes issues when consumers access properties on variables
+    // initialized in setUp blocks within `factory`.
+    group('does not call `factory` directly within the consuming group', () {
+      void sharedTest(
+        BuilderOnlyUiFactory factory, {
+        List Function(PropsMetaCollection) getUnconsumedPropKeys,
+      }) {
+        var wasFactoryCalled = false;
+
+        // These needs to be before declared before commonComponentTests is called,
+        // or these tests may fail when they shouldn't.
+
+        // Test both setUpAll/setUp since they have different timings relative
+        // to commonComponent test setup blocks.
+        //
+        // setUp is the important one, but might as well test both while we're
+        // here!
+
+        setUpAll(() {
+          expect(wasFactoryCalled, isFalse,
+              reason: 'factory arg was called within group, '
+                  'before consumer setUpAll blocks are called');
+        });
+
+        int setUpCallCount = 0;
+        setUp(() {
+          // Only do this the first time, since it gets called before every
+          // test inside commonComponentTests.
+          if (setUpCallCount == 0) {
+            expect(wasFactoryCalled, isFalse,
+                reason: 'factory arg was called within group, '
+                    'before consumer setUp blocks are called');
+          }
+          setUpCallCount++;
+        });
+
+        commonComponentTests(() {
+          wasFactoryCalled = true;
+          return factory();
+        }, getUnconsumedPropKeys: getUnconsumedPropKeys);
+      }
+
+      group('when passed a UiComponent', () {
+        // todo create a new component for this
+        sharedTest(() => TestCommonRequired()
+          ..bar = true
+          ..foobar = true);
+      });
+
+      group('when passed a UiComponent2', () {
+        group('(old boilerplate)', () {
+          // todo create a new component for this
+          sharedTest(() => TestCommonRequired2()
+            ..bar = true
+            ..foobar = true);
+        });
+
+        group('(new boilerplate)', () {
+          // todo create a new component for this
+          sharedTest(new_boilerplate.TestCommonForwarding, getUnconsumedPropKeys: (meta) => [
+            ...meta.forMixin(new_boilerplate.ShouldBeForwardedProps).keys,
+          ]);
+        });
+      });
+    });
   });
 }

test/over_react_test/utils/test_common_component_required_props.dart

@@ -51,6 +51,7 @@ class TestCommonRequiredComponent extends UiComponent<TestCommonRequiredProps> {
   render() {
     return (Dom.div()
       ..addProps(copyUnconsumedDomProps())
+      ..className = forwardingClassNameBuilder().toClassName()
     )(props.children);
   }
 }

test/over_react_test/utils/test_common_component_required_props_commponent2.dart

@@ -49,6 +49,7 @@ class TestCommonRequiredComponent2 extends
   render() {
     return (Dom.div()
       ..modifyProps(addUnconsumedDomProps)
+      ..className = forwardingClassNameBuilder().toClassName()
     )(props.children);
   }
 }