CPLAT-7684: Finalize UiComponent2 Tests by kealjones-wk · Pull Request #348 · Workiva/over_react

kealjones-wk · 2019-10-01T22:58:12Z

Motivation

Review tests for new UiComponent2 functionality, and ensure that there is no missing test coverage, and that all tests for UiComponent have mirrored tests for UiComponent2

AC

over_react
- Review tests for new UiComponent2 functionality, and ensure that there is no missing test coverage, and that all tests for UiComponent have mirrored tests for UiComponent2
- Specifics:
  - Add test coverage that typedPropsFactory/stateFactory handles null by falling back to a new map

Changes

Release Notes

Review

See CONTRIBUTING.md for more details on review types (+1 / QA +1 / +10) and code review process.

Please review:

QA Checklist

Tests were updated and provide good coverage of the changeset and other affected code
Manual testing was performed if needed
- Steps from PR author:
- Anything falling under manual testing criteria outlined in CONTRIBUTING.md

Merge Checklist

While we perform many automated checks before auto-merging, some manual checks are needed:

A Client Platform member has reviewed these changes
There are no unaddressed comments - this check can be automated if reviewers use the "Request Changes" feature
For release PRs - Version metadata in Rosie comment is correct

aviary-wf · 2019-10-01T22:58:36Z

Security Insights

No security relevant content was detected by automated scans.

Action Items

Review PR for security impact; comment "security review required" if needed or unsure
Verify aviary.yaml coverage of security relevant code

Questions or Comments? Reach out on Slack: #support-infosec.

- Deprecate registerAbstractComponent - Add DummyComponent2 - Update some tests

- Add check that validates functionality is retained

…izeState add component_base_2 tests

* 3.1.0-wip: Restore 5.1.0-wip dependency override Update version string Update component references to Component2 Bump lower bound of over_react_test Widen react alpha dependency range Bump lower bound of w_flux Update version string / changelog Remove react dependency override Bump lower bound of react dependency Export TypedSnapshot

aaronlademann-wf · 2019-10-08T17:01:54Z

test/over_react/component_declaration/component_base_test.dart

+      group('`state`', () {
+        group('getter:', () {
+          test('returns a UiState view into the component\'s state map', () {
+            expect(statefulComponent.state, const TypeMatcher<TestStatefulComponent2State>());


Just curious... why not use isA< TestStatefulComponent2State >() instead of const TypeMatcher?

because i didn't write this test and just copy/pasted the tests above it and updated them to use UiComponet2. lol

test/over_react/component_declaration/component_base_test.dart

aaronlademann-wf · 2019-10-08T17:03:51Z

test/over_react/component_declaration/component_base_test.dart

+        expect(newState1, const TypeMatcher<TestStatefulComponent2State>());
+        expect(newState2, const TypeMatcher<TestStatefulComponent2State>());


I feel like we're using isA<...>() elsewhere in Dart2-only libs since it is the recommended replacement for const isInstanceOf<...>().

test/over_react/component_declaration/component2_type_checking_test/test_a.dart

Co-Authored-By: sydneyjodon-wk <51122966+sydneyjodon-wk@users.noreply.github.com> Co-Authored-By: Aaron Lademann <aaron.lademann@workiva.com>

- Long Live isA!

…/over_react into component2-test-coverage

* 3.1.0-wip: Uncomment built_value_generator Bump analyzer top constraint over_react 3.0.0-alpha.1+dart2 Improve readability of large function body Fix null aware assignment typo Move var closer to usage Reset internal error tracking immediately Address documentation CR feedback Use a managed timer Update SvgProp typing Change open to a bool Improve handling of “repeat” errors that can lock up the main thread Don't use package for builder import make local builder private

greglittlefield-wf · 2019-10-15T17:35:49Z

lib/src/component/dummy_component2.dart

+// UiComponent2Bridge and it's jsifyPropTypes implementation require a UiComponent2,
+// and cannot simply be a react.Component2 class.
+//
+// This is to publicly expose the generated component type for use with `registerAbstractComponent2`.


Can we make Dummy2Component and its props/factory private, then?

grosss lol. shitty boilerplate ftl

greglittlefield-wf · 2019-10-15T17:41:06Z

test/over_react/component_declaration/component_type_checking_test.dart

+}
+
+testComponentTypeChecking({
+  isComponent2: false,


This should have typing

Suggested change

isComponent2: false,

bool isComponent2: false,

greglittlefield-wf · 2019-10-15T17:41:54Z

test/over_react/component_declaration/component_type_checking_test.dart

+
+testComponentTypeChecking({
+  isComponent2: false,
+  TestParent,


These should also be typed

Suggested change

TestParent,

UiFactory TestParent,

and so on

greglittlefield-wf · 2019-10-15T17:44:19Z

test/test_util/component2/one_level_wrapper.dart

-@Component(isWrapper: true)
-class OneLevelWrapper2Component extends UiComponent<OneLevelWrapper2Props> {
+@Component2(isWrapper: true)
+class OneLevelWrapper2Component extends UiComponent2<OneLevelWrapper2Props> {


😬 Nice catch!!

lulz thanks

greglittlefield-wf · 2019-10-15T17:44:44Z

test/vm_tests/builder/declaration_parsing_test.dart

              expect(declarations.state.meta.keyNamespace, 'baz');
              expect(declarations.component.meta.isWrapper, isTrue);
            });
+            test('a stateful Component2', () {


#nit could use a newline before this

Suggested change

test('a stateful Component2', () {

test('a stateful Component2', () {

greglittlefield-wf

+10

greglittlefield-wf · 2019-10-16T00:13:04Z

@Workiva/release-management-p

kealjones-wk · 2019-10-16T00:33:48Z

+1

more component2 test coverage

55cfd97

rmconsole-wf added the Merge Requirements Unmet label Oct 1, 2019

kealjones-wk added 8 commits October 2, 2019 13:59

only my changes

787b259

fix ending line

3e57ed5

Add registerAbstractComponent2

92dbf57

- Deprecate registerAbstractComponent - Add DummyComponent2 - Update some tests

add final newline to dummy_component

25d2d28

fix doc comment for registerAbstractComponent2

29aa748

fix extra newlines

40ea738

cleanup dummy_component2

9a17aaa

add typed(Props|State)Factory null tests

82bcacf

kealjones-wk changed the title ~~CPLAT-2276: Finalize ComponentV2 / UiComponentV2 Tests~~ CPLAT-2276: Finalize UiComponent2 Tests Oct 4, 2019

update typed(Props|State)Factory tests

d08f31a

- Add check that validates functionality is retained

kealjones-wk changed the title ~~CPLAT-2276: Finalize UiComponent2 Tests~~ CPLAT-7684: Finalize UiComponent2 Tests Oct 7, 2019

Update outdated generated state setter assert that referenced initial…

ce35aa6

…izeState add component_base_2 tests

kealjones-wk marked this pull request as ready for review October 8, 2019 00:00

kealjones-wk added 2 commits October 7, 2019 17:08

update generated files... again?

c6a46f5

aaronlademann-wf reviewed Oct 8, 2019

View reviewed changes

sydneyjodon-wk reviewed Oct 9, 2019

View reviewed changes

test/over_react/component_declaration/component2_type_checking_test/test_a.dart Outdated Show resolved Hide resolved

kealjones-wk and others added 6 commits October 10, 2019 15:05

Apply suggestions from code review

07531dc

Co-Authored-By: sydneyjodon-wk <51122966+sydneyjodon-wk@users.noreply.github.com> Co-Authored-By: Aaron Lademann <aaron.lademann@workiva.com>

We dont like TypeMatcher anymore...

cc6b4d7

- Long Live isA!

Merge branch 'component2-test-coverage' of https://github.com/Workiva…

8a7e5af

…/over_react into component2-test-coverage

regenerate files

e8ced3b

Merge branch '3.1.0-wip' into component2-test-coverage

ed2d555

greglittlefield-wf reviewed Oct 15, 2019

View reviewed changes

address pr feedback

0101f47

greglittlefield-wf approved these changes Oct 15, 2019

View reviewed changes

rmconsole-wf added the RM Ready label Oct 16, 2019

rmconsole-wf added Merge Requirements Met and removed Merge Requirements Unmet labels Oct 16, 2019

rmconsole4-wk merged commit 683f7d3 into 3.1.0-wip Oct 16, 2019

rmconsole4-wk deleted the component2-test-coverage branch October 16, 2019 00:34

		expect(newState1, const TypeMatcher<TestStatefulComponent2State>());
		expect(newState2, const TypeMatcher<TestStatefulComponent2State>());

	test('a stateful Component2', () {

	test('a stateful Component2', () {

Conversation

kealjones-wk commented Oct 1, 2019

Motivation

AC

Changes

Release Notes

Review

QA Checklist

Merge Checklist

Uh oh!

aviary-wf commented Oct 1, 2019

Security Insights

Action Items

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

greglittlefield-wf left a comment

Choose a reason for hiding this comment

Uh oh!

greglittlefield-wf commented Oct 16, 2019

Uh oh!

kealjones-wk commented Oct 16, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants