UpdateLog tests #1

rauanmayemir · 2018-04-12T15:29:12Z

This introduces UpdateLog tests. UpdateLog is a data structure which contains a description of mutations to the instance tree in order to get to the desired state.

The most important part is subTreeChange which highlights the type of subtree change.

Nested means that the underlying subtree did change but it was an element deep down the tree.
NoChange means that the tree has not changed.
ReplaceElements means that a subtree should be replaced
PrependElement means that an element should be prepended

Todos:

Improve formatting in tests. I want to handle this first since poor readability affects productivity a lot
Determine why the existing test is failing :)
Implement tests which verify the UpdateLog step by step. It's important since we can determine that the minimal amount of steps is indeed in the UpdateLog.
~~Test applyUpdateLog. The tree from applyUpdateLog should be the same as newly returned element.~~
Introduce side effects in applyUpdateLog
applyUpdateLog should perform mutations to the tree and invoke render-cycle lifecycle functions. In order to test that we should build a list of all mutations and lifecycle function invocations and assert if they happen. Similar to what we do with UpdateLog. The RenderUpdateLog however will only be a part of tests. Also, in order to test it properly we need to introduce mutations to the TestRenderer because currently the hostView is immutable.
add tests for prependElement
~~Remember to include updateInstance in the tests~~ Test `ContentChanged

Determining the subtree change at the reconciliation time lets us perform minimal changes to the underlying views. It also introduces formatting testInstances as JSX

wokalski · 2018-04-12T15:44:23Z

I think we should also squash [294ab97... f17cd5e] Into Add SwitchComponent to UpdateLog and introduce topLevelUpdate.

wokalski · 2018-04-12T15:45:28Z

ec8e2bd -> ReactCore refactor?

This commit also includes some changes in the algorithm to ignore empty updates.

They are included inside new/old instance

Instances are now memoized by the library

wokalski

There's one conclusion from this: we've got bugs. There's also another conclusion, there are parts I don't understand. I believe these are not very simple fixes but they are necessary.

wokalski · 2018-04-13T15:26:33Z

test/Test.re

+            ],
+            Some(
+              TestRenderer.{
+                subtreeChange: `Nested,


This doesn't make sense. If this caused `Nested, what triggers ReplaceElements for the top level update?

If I'm understanding this correctly, ReplaceElements is triggered only when we're replacing children element with a list of new elements (or inserting a list of new elements into the empty subtree).

I suggest we do a complete overhaul of UpdateLog types and outline all possible scenarios how updates could happen. (we could reuse our existing test suites establishing the correct expected output and work towards making the actual output match)

I applaud this suggestion.

wokalski · 2018-04-13T15:33:57Z

test/Test.re

+                  ref([
+                    UpdateInstance({
+                      stateChanged: true,
+                      subTreeChanged: `NoChange,


This doesn't make sense - the subtree didn't change but the content has changed. It'll probably require something like shouldUpdateContent on the nativeElement... (Although it'd be awesome to avoid that.)

wokalski · 2018-04-13T15:38:20Z

test/Test.re

-        elem
+    ),
+    (
+      "Test Lists With Dynamic Keys",


What are dynamic keys?

wokalski · 2018-04-13T15:42:06Z

test/Test.re

+                subTreeChanged:
+                  `ReplaceElements((
+                    [<Box id=2 state="Hello" />],
+                    [<Box id=3 state="World" />, <Box id=4 state="Hello" />]


Shouldn't this be PrependElement? If not, why? Actually, it shouldn't be, but why not? We need to document this.

wokalski · 2018-04-13T15:44:33Z

test/Test.re

+      }
+    ),
+    (
+      "Deep Move Box With Dynamic Keys",


Interesting... but how it happens?

wokalski · 2018-04-13T15:45:06Z

test/Test.re

-        TestRenderer.update(
-          rendered3,
-          <ButtonWrapperWrapper wrappedText="updatedTextmodified" />
+        /* TODO Compare rendered0 and rendered1 */


It seems like an unfinished test.

wokalski · 2018-04-13T15:47:24Z

test/Test.re

+                  ref([
+                    UpdateInstance({
+                      stateChanged: true,
+                      subTreeChanged: `NoChange,


Again, this should yield that the context has changed.

wokalski · 2018-04-13T15:48:35Z

test/Test.re

+            ],
+            Some(
+              TestRenderer.{
+                subtreeChange: `Nested,


What we've done in this test is reversing the order of boxes. It's not reflected in the subtreeChange... It'd cause lack of rerender/relayout which in turn would cause a visual bug. How will we solve this?

wokalski · 2018-04-13T15:52:29Z

test/Test.re

+        TestRenderer.convertElement(rendered)
+        |> check(renderedElement, "Initial", result(~state="0", ~text="0"));
+        RemoteAction.act(rAction, ~action=Click);
+        let (rendered, _) = RenderedElement.flushPendingUpdates(rendered);


we're not checking the updateLog here.

Flat now contains only one element

…ment fixes

wokalski · 2018-04-20T18:09:31Z

I just added a test for swapping elements. It's currently failing. The fix is fairly easy but requires changes to almost all tests (because now Nested and NoChange must also carry information if there's been some reordering (like Nested(true)).
I think we should remove useDynamicKey. I don't understand its usefulness
After fixing the reordering situation this PR should be feature wise good to go.

test printer minor fixes

fix output diffing, improve indentation

wokalski · 2018-05-15T07:45:14Z

@rauanmayemir After refactoring the tests to the new DSL we should remove Assert module or rather merge it with the Run module.

wokalski added 2 commits February 23, 2018 20:52

First tests for UpdateLog

cb8900f

[No tests] Make subTreeChanged describe the type of change

da697d3

Determining the subtree change at the reconciliation time lets us perform minimal changes to the underlying views. It also introduces formatting testInstances as JSX

rauanmayemir and others added 19 commits April 12, 2018 17:48

Restructure dirs, module names, jbuild files

8ef1858

Add custom assert check for tests

a9fa83c

Little test formatting fix

a3ad6d3

Added subtree change comparison func, fixed top level update tests

1a07d31

[Failing] Replace NewRenderedElement with TopLevelUpdate

ec1677c

This commit also includes some changes in the algorithm to ignore empty updates.

Remove setProps and layout from nativeElement

1c253e7

Make updateable components stateful

6bbb896

Reorganize tests into separate modules

75f672b

Factor out test printers

959c487

Remove the distinction between NativeInstance and Instance

25e9944

Add more tests of UpdateLogs

d4ad86e

Introduce SwitchComponent UpdateLog and add topLevelUpdate

69bbedf

ReactCore refactor (renaming and removing duplication)

02482da

Fix incorrect oldInstance passed to UpdateInstance

c7896f1

Fix element comparators in tests

8ec580b

Remove old and new Subtree from ChangeComponent

d563963

They are included inside new/old instance

Remove Layout from ReactCore

8886e7f

Remove id from nativeElement.make.

30f9ab1

Instances are now memoized by the library

Delete some unnecessary code from tests

04e25bf

wokalski force-pushed the tests branch from 698e477 to 04e25bf Compare April 12, 2018 16:05

wokalski added 2 commits April 12, 2018 18:22

Remove a stale comment and OutputTree module

72e02cf

Remove iOS related files

adb2d44

wokalski force-pushed the tests branch from 675067b to adb2d44 Compare April 12, 2018 16:22

wokalski requested changes Apr 13, 2018

View reviewed changes

wokalski added 2 commits April 18, 2018 05:45

Remove the iOS project

cc33893

Add shouldContentUpdate to nativeElement and simplify Flat/Nested

60a2cca

Flat now contains only one element

rauanmayemir and others added 4 commits April 18, 2018 16:25

test components clean up

dee32b4

remove processElement function and unused positional check, minor com…

b1dba46

…ment fixes

Add ContentChanged subtree change

7817b51

Add a failing test for swapping elements

0ab83e0

rauanmayemir added 2 commits May 2, 2018 19:54

Add easy-format pretty printers for test components and types

ff5dc1f

test printer minor fixes

Add diffing of expected/actual test outputs

708ed67

fix output diffing, improve indentation

rauanmayemir force-pushed the tests branch from 4f58110 to 708ed67 Compare May 2, 2018 13:56

Add `Reordered subtree change

86863fb

rauanmayemir force-pushed the tests branch from 4b8a52a to 86863fb Compare May 11, 2018 13:27

rauanmayemir and others added 3 commits May 11, 2018 17:08

Optimize `Reordered check, account for top level flat update

7d2820e

Add a test for prepending an element

7d2abbb

Introduce DSL for tests and migrate a first part of the tests to it

f60f5bf

rauanmayemir added 4 commits May 15, 2018 22:43

Migrate remote action tests suite to new test dsl

186cc59

Migrate all test suites to new DSL, restructure test helper modules

8f80d95

Refactor tests, review naming and labels, improve dynamic key intuition

98b08d6

Rename BoxTable to BoxList

52be6dd

wokalski approved these changes May 18, 2018

View reviewed changes

wokalski merged commit 5d57787 into master May 18, 2018

wokalski deleted the tests branch May 18, 2018 18:49

rauanmayemir changed the title ~~[WIP] UpdateLog tests~~ UpdateLog tests May 23, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UpdateLog tests #1

UpdateLog tests #1

rauanmayemir commented Apr 12, 2018 •

edited by wokalski

Loading

wokalski commented Apr 12, 2018

wokalski commented Apr 12, 2018

wokalski left a comment

wokalski Apr 13, 2018

rauanmayemir Apr 14, 2018

wokalski Apr 14, 2018

wokalski Apr 13, 2018

wokalski Apr 13, 2018

wokalski Apr 13, 2018

wokalski Apr 13, 2018

wokalski Apr 13, 2018

wokalski Apr 13, 2018

wokalski Apr 13, 2018

wokalski Apr 13, 2018

wokalski commented Apr 20, 2018

wokalski commented May 15, 2018

UpdateLog tests #1

UpdateLog tests #1

Conversation

rauanmayemir commented Apr 12, 2018 • edited by wokalski Loading

wokalski commented Apr 12, 2018

wokalski commented Apr 12, 2018

wokalski left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wokalski commented Apr 20, 2018

wokalski commented May 15, 2018

rauanmayemir commented Apr 12, 2018 •

edited by wokalski

Loading