You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@flex.apache.org by mi...@apache.org on 2017/08/16 09:52:17 UTC
[1/3] git commit: [flex-sdk] [refs/heads/develop] - FLEX-18746 Added
the same test,
but without using opening animation. (This way the bug cannot be reproduced,
which is a good clue.)
Repository: flex-sdk
Updated Branches:
refs/heads/develop b175cd6d2 -> fcc25865f
FLEX-18746 Added the same test, but without using opening animation. (This way the bug cannot be reproduced, which is a good clue.)
Project: http://git-wip-us.apache.org/repos/asf/flex-sdk/repo
Commit: http://git-wip-us.apache.org/repos/asf/flex-sdk/commit/7a519a33
Tree: http://git-wip-us.apache.org/repos/asf/flex-sdk/tree/7a519a33
Diff: http://git-wip-us.apache.org/repos/asf/flex-sdk/diff/7a519a33
Branch: refs/heads/develop
Commit: 7a519a33e4e93d9657531b15e24d5684a0f80abc
Parents: b175cd6
Author: Mihai Chira <mi...@apache.org>
Authored: Tue Aug 15 12:19:14 2017 +0200
Committer: Mihai Chira <mi...@apache.org>
Committed: Tue Aug 15 12:19:14 2017 +0200
----------------------------------------------------------------------
.../tests/mx/controls/Tree_FLEX_18746_Tests.as | 39 +++++++++++++++-----
1 file changed, 30 insertions(+), 9 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/flex-sdk/blob/7a519a33/frameworks/projects/mx/tests/mx/controls/Tree_FLEX_18746_Tests.as
----------------------------------------------------------------------
diff --git a/frameworks/projects/mx/tests/mx/controls/Tree_FLEX_18746_Tests.as b/frameworks/projects/mx/tests/mx/controls/Tree_FLEX_18746_Tests.as
index 26568d6..16a3cbc 100644
--- a/frameworks/projects/mx/tests/mx/controls/Tree_FLEX_18746_Tests.as
+++ b/frameworks/projects/mx/tests/mx/controls/Tree_FLEX_18746_Tests.as
@@ -18,8 +18,8 @@ package mx.controls {
private static var _sut:Tree;
private static var child:Object = {label: "Item"};
- private static var parent0:Object = {label: "Folder 0", children: new ArrayCollection()};
- private static var parent1:Object = {label: "Folder 1", children: new ArrayCollection([child])};
+ private static var parent0:Object;
+ private static var parent1:Object;
[Before]
@@ -28,6 +28,10 @@ package mx.controls {
_sut = new Tree();
_sut.width = 200;
_sut.height = 200;
+
+ parent0 = {label: "Folder 0", children: new ArrayCollection()};
+ parent1 = {label: "Folder 1", children: new ArrayCollection([child])};
+
UIImpersonator.addChild(_sut);
}
@@ -46,7 +50,24 @@ package mx.controls {
//--------------------------------------------------------------------------
[Test(async, timeout=1000)]
- public function test_closing_previously_opened_folder_with_0_children_does_not_throw_fatal():void
+ public function test_closing_previously_opened_folder_with_0_children_without_animation_does_not_throw_fatal():void
+ {
+ //given
+ const dataProvider:ArrayCollection = new ArrayCollection();
+ dataProvider.addItem(parent0);
+ dataProvider.addItem(parent1);
+
+ //when
+ _sut.dataProvider = dataProvider;
+
+ //then wait a few frames
+ noEnterFramesToWait = 2;
+ UIImpersonator.testDisplay.addEventListener(Event.ENTER_FRAME, onEnterFrame);
+ Async.handleEvent(this, _finishNotifier, Event.COMPLETE, then_expand_second_folder, 300, {useAnimation:false});
+ }
+
+ [Test(async, timeout=1000)]
+ public function test_closing_previously_opened_folder_with_0_children_using_animation_does_not_throw_fatal():void
{
//given
const dataProvider:ArrayCollection = new ArrayCollection();
@@ -59,19 +80,19 @@ package mx.controls {
//then wait a few frames
noEnterFramesToWait = 2;
UIImpersonator.testDisplay.addEventListener(Event.ENTER_FRAME, onEnterFrame);
- Async.handleEvent(this, _finishNotifier, Event.COMPLETE, then_expand_second_folder, 300);
+ Async.handleEvent(this, _finishNotifier, Event.COMPLETE, then_expand_second_folder, 300, {useAnimation:true});
}
private function then_expand_second_folder(event:Event, passThroughData:Object):void
{
//when
- _sut.expandItem(parent1, true, true, true);
+ _sut.expandItem(parent1, true, passThroughData.useAnimation, true);
//then wait a bit
noEnterFramesToWait = 5;
UIImpersonator.testDisplay.addEventListener(Event.ENTER_FRAME, onEnterFrame);
- Async.handleEvent(this, _finishNotifier, Event.COMPLETE, then_move_child_to_first_parent_and_expand_it, 500);
+ Async.handleEvent(this, _finishNotifier, Event.COMPLETE, then_move_child_to_first_parent_and_expand_it, 500, passThroughData);
}
private function then_move_child_to_first_parent_and_expand_it(event:Event, passThroughData:Object):void
@@ -81,19 +102,19 @@ package mx.controls {
//when
ArrayCollection(parent1.children).removeItemAt(0);
- _sut.expandItem(parent0, true, true, true);
+ _sut.expandItem(parent0, true, passThroughData.useAnimation, true);
ArrayCollection(parent0.children).addItem(child);
//then wait a bit
noEnterFramesToWait = 1;
UIImpersonator.testDisplay.addEventListener(Event.ENTER_FRAME, onEnterFrame);
- Async.handleEvent(this, _finishNotifier, Event.COMPLETE, then_contract_second_folder, 200);
+ Async.handleEvent(this, _finishNotifier, Event.COMPLETE, then_contract_second_folder, 200, passThroughData);
}
private static function then_contract_second_folder(event:Event, passThroughData:Object):void
{
//when
- _sut.expandItem(parent1, false, true, true);
+ _sut.expandItem(parent1, false, passThroughData.useAnimation, true);
//then no error was thrown
assertThat(true);
[2/3] git commit: [flex-sdk] [refs/heads/develop] - FLEX-18746 Adding
another unit test which goes to the heart of the bug - the length calculation
in HierarchicalCollectionView. (Currently the test function
test_opening_closing_with_Jill_having_no_child
Posted by mi...@apache.org.
FLEX-18746 Adding another unit test which goes to the heart of the bug - the length calculation in HierarchicalCollectionView. (Currently the test function test_opening_closing_with_Jill_having_no_children_to_begin_with() fails, as expected.)
Project: http://git-wip-us.apache.org/repos/asf/flex-sdk/repo
Commit: http://git-wip-us.apache.org/repos/asf/flex-sdk/commit/5fb2fb63
Tree: http://git-wip-us.apache.org/repos/asf/flex-sdk/tree/5fb2fb63
Diff: http://git-wip-us.apache.org/repos/asf/flex-sdk/diff/5fb2fb63
Branch: refs/heads/develop
Commit: 5fb2fb634ea856cc0cd4034dc9bc99e4a58219d5
Parents: 7a519a3
Author: Mihai Chira <mi...@apache.org>
Authored: Tue Aug 15 17:20:48 2017 +0200
Committer: Mihai Chira <mi...@apache.org>
Committed: Tue Aug 15 17:20:48 2017 +0200
----------------------------------------------------------------------
.../Tree_FLEX_18746_Collection_Length_Tests.as | 201 +++++++++++++++++++
.../tests/mx/controls/Tree_FLEX_18746_Tests.as | 6 -
2 files changed, 201 insertions(+), 6 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/flex-sdk/blob/5fb2fb63/frameworks/projects/mx/tests/mx/controls/Tree_FLEX_18746_Collection_Length_Tests.as
----------------------------------------------------------------------
diff --git a/frameworks/projects/mx/tests/mx/controls/Tree_FLEX_18746_Collection_Length_Tests.as b/frameworks/projects/mx/tests/mx/controls/Tree_FLEX_18746_Collection_Length_Tests.as
new file mode 100644
index 0000000..1d7a131
--- /dev/null
+++ b/frameworks/projects/mx/tests/mx/controls/Tree_FLEX_18746_Collection_Length_Tests.as
@@ -0,0 +1,201 @@
+package mx.controls {
+ import flash.events.Event;
+ import flash.events.EventDispatcher;
+
+ import mx.collections.ArrayCollection;
+ import mx.core.mx_internal;
+
+ import org.flexunit.asserts.assertEquals;
+ import org.flexunit.async.Async;
+ import org.fluint.uiImpersonation.UIImpersonator;
+
+ use namespace mx_internal;
+
+ public class Tree_FLEX_18746_Collection_Length_Tests
+ {
+ private static var noEnterFramesToWait:int = NaN;
+ private static const _finishNotifier:EventDispatcher = new EventDispatcher();
+
+ private static var _sut:Tree_;
+ private static var Sam:TreeItem;
+ private static var Ana:TreeItem;
+ private static var Jenny:TreeItem;
+ private static var Marc:TreeItem;
+ private static var parentJill:TreeItem;
+ private static var parentJohn:TreeItem;
+
+
+ [Before]
+ public function setUp():void
+ {
+ Sam = new TreeItem("Sam");
+ Ana = new TreeItem("Ana");
+ Jenny = new TreeItem("Jenny");
+ Marc = new TreeItem("Marc");
+
+ _sut = new Tree_();
+ _sut.width = 200;
+ _sut.height = 200;
+
+ UIImpersonator.addChild(_sut);
+ }
+
+ [After]
+ public function tearDown():void
+ {
+ UIImpersonator.removeAllChildren();
+ _sut = null;
+ }
+
+
+ [Test(async, timeout=300)]
+ public function test_opening_closing_with_both_parents_having_at_least_one_child():void
+ {
+ //given
+ parentJill = new TreeItem("Jill", new ArrayCollection([Marc]));
+ parentJohn = new TreeItem("John", new ArrayCollection([Sam]));
+
+ const dataProvider:ArrayCollection = new ArrayCollection();
+ dataProvider.addItem(parentJill);
+ dataProvider.addItem(parentJohn);
+
+ //when
+ _sut.dataProvider = dataProvider;
+
+ //then wait a few frames
+ noEnterFramesToWait = 2;
+ UIImpersonator.testDisplay.addEventListener(Event.ENTER_FRAME, onEnterFrame);
+ Async.handleEvent(this, _finishNotifier, Event.COMPLETE, then_expand_and_contract, 250);
+ }
+
+ [Test(async, timeout=300)]
+ public function test_opening_closing_with_Jill_having_no_children_to_begin_with():void
+ {
+ //given
+ parentJill = new TreeItem("Jill", new ArrayCollection());
+ parentJohn = new TreeItem("John", new ArrayCollection([Sam]));
+
+ const dataProvider:ArrayCollection = new ArrayCollection();
+ dataProvider.addItem(parentJill);
+ dataProvider.addItem(parentJohn);
+
+ //when
+ _sut.dataProvider = dataProvider;
+
+ //then wait a few frames
+ noEnterFramesToWait = 2;
+ UIImpersonator.testDisplay.addEventListener(Event.ENTER_FRAME, onEnterFrame);
+ Async.handleEvent(this, _finishNotifier, Event.COMPLETE, then_expand_and_contract, 250);
+ }
+
+ private static function then_expand_and_contract(event:Event, passThroughData:Object):void
+ {
+ //given
+ var currentLength:int = _sut.collectionLength_; //current length is correct
+
+ //then
+ assertEquals(2, currentLength);
+
+ //when
+ _sut.expandItem(parentJohn, true, false, true);
+ currentLength += parentJohn.children.length;
+
+ //then
+ assertEquals(currentLength, _sut.collectionLength_);
+
+ //when
+ _sut.expandItem(parentJill, true, false, true);
+ currentLength += parentJill.children.length;
+
+ //then
+ assertEquals(currentLength, _sut.collectionLength_);
+
+ //when
+ parentJohn.children.addItem(Jenny);
+ currentLength += 1;
+
+ //then
+ assertEquals(currentLength, _sut.collectionLength_);
+
+ //when
+ _sut.expandItem(parentJohn, false, false, true);
+ currentLength -= parentJohn.children.length;
+
+ //then
+ assertEquals(currentLength, _sut.collectionLength_);
+
+ //when
+ parentJill.children.addItem(Ana);
+ currentLength += 1;
+
+ //then
+ assertEquals(currentLength, _sut.collectionLength_);
+ }
+
+
+ private static function onEnterFrame(event:Event):void
+ {
+ if(!--noEnterFramesToWait)
+ {
+ UIImpersonator.testDisplay.removeEventListener(Event.ENTER_FRAME, onEnterFrame);
+ _finishNotifier.dispatchEvent(new Event(Event.COMPLETE));
+ }
+ }
+ }
+}
+
+import mx.controls.Tree;
+import mx.controls.treeClasses.HierarchicalCollectionView;
+
+class Tree_ extends Tree
+{
+ public function getHierarchicalCollection():HierarchicalCollectionView
+ {
+ return super.collection as HierarchicalCollectionView;
+ }
+
+ public function get collectionLength_():int
+ {
+ return getHierarchicalCollection().length;
+ }
+}
+
+import mx.collections.ArrayCollection;
+
+class TreeItem {
+ private var _label:String;
+ private var _children:ArrayCollection;
+
+ public function TreeItem(label:String, children:ArrayCollection = null)
+ {
+ this.label = label;
+ this.children = children;
+ }
+
+ [Bindable]
+ public function set label(label:String):void
+ {
+ _label = label;
+ }
+
+ public function get label():String
+ {
+ return _label;
+ }
+
+ [Bindable]
+ public function set children(children:ArrayCollection):void
+ {
+ _children = children;
+ }
+
+ public function get children():ArrayCollection
+ {
+ return _children;
+ }
+
+ public function toString():String
+ {
+ return "TreeItem{_label=" + String(_label) + "}";
+ }
+}
\ No newline at end of file
http://git-wip-us.apache.org/repos/asf/flex-sdk/blob/5fb2fb63/frameworks/projects/mx/tests/mx/controls/Tree_FLEX_18746_Tests.as
----------------------------------------------------------------------
diff --git a/frameworks/projects/mx/tests/mx/controls/Tree_FLEX_18746_Tests.as b/frameworks/projects/mx/tests/mx/controls/Tree_FLEX_18746_Tests.as
index 16a3cbc..0cfb3af 100644
--- a/frameworks/projects/mx/tests/mx/controls/Tree_FLEX_18746_Tests.as
+++ b/frameworks/projects/mx/tests/mx/controls/Tree_FLEX_18746_Tests.as
@@ -43,12 +43,6 @@ package mx.controls {
}
- //--------------------------------------------------------------------------
- //
- // Test method
- //
- //--------------------------------------------------------------------------
-
[Test(async, timeout=1000)]
public function test_closing_previously_opened_folder_with_0_children_without_animation_does_not_throw_fatal():void
{
[3/3] git commit: [flex-sdk] [refs/heads/develop] - FLEX-18746 CAUSE:
HierarchicalCollectionView.updateLength() was ignoring nodes without children,
as it should. However, the function it called for the nodes with children,
getChildren() unfortunately ha
Posted by mi...@apache.org.
FLEX-18746 CAUSE: HierarchicalCollectionView.updateLength() was ignoring nodes without children, as it should. However, the function it called for the nodes with children, getChildren() unfortunately had a necessary side-effect (which is a poor design choice, to be sure), which was to add an event listener to the children collection. So the empty collections of nodes were not listened to, which meant that once the node was open any children that were added to that node were not counted towards the length of the collection. And one way this inconsistency surfaced was through the expandItem() fatal.
SOLUTION: The best - and most time-consuming - solution to this is to find a way to extract the side-effect of getChildren() into a separate function and call it when needed. However, for the moment I have opted to simply allow getChildren() to be called even for empty nodes, which in turn adds the event listeners.
NOTES: all unit tests now pass.
Project: http://git-wip-us.apache.org/repos/asf/flex-sdk/repo
Commit: http://git-wip-us.apache.org/repos/asf/flex-sdk/commit/fcc25865
Tree: http://git-wip-us.apache.org/repos/asf/flex-sdk/tree/fcc25865
Diff: http://git-wip-us.apache.org/repos/asf/flex-sdk/diff/fcc25865
Branch: refs/heads/develop
Commit: fcc25865f43a7f0760da5a5b1849a932349797d8
Parents: 5fb2fb6
Author: Mihai Chira <mi...@apache.org>
Authored: Wed Aug 16 11:51:05 2017 +0200
Committer: Mihai Chira <mi...@apache.org>
Committed: Wed Aug 16 11:51:05 2017 +0200
----------------------------------------------------------------------
frameworks/projects/mx/src/mx/controls/Tree.as | 2 +-
.../src/mx/controls/treeClasses/HierarchicalCollectionView.as | 7 +++----
2 files changed, 4 insertions(+), 5 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/flex-sdk/blob/fcc25865/frameworks/projects/mx/src/mx/controls/Tree.as
----------------------------------------------------------------------
diff --git a/frameworks/projects/mx/src/mx/controls/Tree.as b/frameworks/projects/mx/src/mx/controls/Tree.as
index 5f9215a..5bf874f 100644
--- a/frameworks/projects/mx/src/mx/controls/Tree.as
+++ b/frameworks/projects/mx/src/mx/controls/Tree.as
@@ -1783,7 +1783,7 @@ public class Tree extends List implements IIMESupport
// is the item on screen?
if (visibleData[uid])
{
- //find the rowindex of the row after the open thats opening/closing
+ //find the row index of the first row after the one that's opening/closing
var n:int = listItems.length;
for (rowIndex = 0; rowIndex < n; rowIndex++)
{
http://git-wip-us.apache.org/repos/asf/flex-sdk/blob/fcc25865/frameworks/projects/mx/src/mx/controls/treeClasses/HierarchicalCollectionView.as
----------------------------------------------------------------------
diff --git a/frameworks/projects/mx/src/mx/controls/treeClasses/HierarchicalCollectionView.as b/frameworks/projects/mx/src/mx/controls/treeClasses/HierarchicalCollectionView.as
index 5f357b0..59fe7e8 100644
--- a/frameworks/projects/mx/src/mx/controls/treeClasses/HierarchicalCollectionView.as
+++ b/frameworks/projects/mx/src/mx/controls/treeClasses/HierarchicalCollectionView.as
@@ -267,7 +267,7 @@ public class HierarchicalCollectionView extends EventDispatcher
var modelCursor:IViewCursor = treeData.createCursor();
if (modelCursor.beforeFirst)
{
- // indicates that an IPE occured on the first item
+ // indicates that an IPE occurred on the first item
return treeData.length;
}
while (!modelCursor.afterLast)
@@ -315,8 +315,7 @@ public class HierarchicalCollectionView extends EventDispatcher
parentMap[uid] = parent;
if (node != null &&
openNodes[uid] &&
- dataDescriptor.isBranch(node, treeData) &&
- dataDescriptor.hasChildren(node, treeData))
+ dataDescriptor.isBranch(node, treeData))
{
childNodes = getChildren(node);
if (childNodes != null)
@@ -432,7 +431,7 @@ public class HierarchicalCollectionView extends EventDispatcher
/**
* @private
- * delegate getchildren in order to add event listeners for nested collections
+ * delegate getChildren in order to add event listeners for nested collections
*/
private function getChildren(node:Object):ICollectionView
{