Return-Path: X-Original-To: apmail-flex-commits-archive@www.apache.org Delivered-To: apmail-flex-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id ACD1617CDB for ; Fri, 20 Mar 2015 08:51:46 +0000 (UTC) Received: (qmail 77804 invoked by uid 500); 20 Mar 2015 08:51:46 -0000 Delivered-To: apmail-flex-commits-archive@flex.apache.org Received: (qmail 77686 invoked by uid 500); 20 Mar 2015 08:51:46 -0000 Mailing-List: contact commits-help@flex.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@flex.apache.org Delivered-To: mailing list commits@flex.apache.org Received: (qmail 77419 invoked by uid 99); 20 Mar 2015 08:51:46 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 20 Mar 2015 08:51:46 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id DA666E193B; Fri, 20 Mar 2015 08:51:45 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: erikdebruin@apache.org To: commits@flex.apache.org Date: Fri, 20 Mar 2015 08:51:46 -0000 Message-Id: In-Reply-To: <79d4b77c7cfa4e5a844311effaad2d09@git.apache.org> References: <79d4b77c7cfa4e5a844311effaad2d09@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [02/15] git commit: [flex-sdk] [refs/heads/release4.14.1] - FLEX-34778 Adding a unit test which verifies that replacements in the HierarchicalCollectionView work as expected in various circumstances, so that a fix for this bug can be made knowing that we FLEX-34778 Adding a unit test which verifies that replacements in the HierarchicalCollectionView work as expected in various circumstances, so that a fix for this bug can be made knowing that we're not breaking something else. Project: http://git-wip-us.apache.org/repos/asf/flex-sdk/repo Commit: http://git-wip-us.apache.org/repos/asf/flex-sdk/commit/83e4a9b2 Tree: http://git-wip-us.apache.org/repos/asf/flex-sdk/tree/83e4a9b2 Diff: http://git-wip-us.apache.org/repos/asf/flex-sdk/diff/83e4a9b2 Branch: refs/heads/release4.14.1 Commit: 83e4a9b21d9592ada8ba0ae38619acad070373e2 Parents: 3ddcfee Author: Mihai Chira Authored: Tue Mar 10 12:37:59 2015 +0100 Committer: Erik de Bruin Committed: Fri Mar 20 09:51:21 2015 +0100 ---------------------------------------------------------------------- .../tests/unitTests/mx/collections/DataNode.as | 107 +++--- .../HierarchicalCollectionViewTestUtils.as | 1 + ...erarchicalCollectionView_FLEX_34778_Tests.as | 15 +- .../HierarchicalCollectionView_REPLACE_Tests.as | 325 +++++++++++++++++++ 4 files changed, 402 insertions(+), 46 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/flex-sdk/blob/83e4a9b2/frameworks/tests/unitTests/mx/collections/DataNode.as ---------------------------------------------------------------------- diff --git a/frameworks/tests/unitTests/mx/collections/DataNode.as b/frameworks/tests/unitTests/mx/collections/DataNode.as index aef1829..730825a 100644 --- a/frameworks/tests/unitTests/mx/collections/DataNode.as +++ b/frameworks/tests/unitTests/mx/collections/DataNode.as @@ -18,64 +18,83 @@ //////////////////////////////////////////////////////////////////////////////// package mx.collections { -import mx.collections.ArrayCollection; + import mx.collections.ArrayCollection; -public class DataNode { - private var _label:String; - private var _children:ArrayCollection; - private var _isSelected:Boolean = false; - private var _isPreviousSiblingRemoved:Boolean = false; + public class DataNode { + private var _label:String; + private var _children:ArrayCollection; + private var _isSelected:Boolean = false; + private var _isPreviousSiblingRemoved:Boolean = false; + private var _parent:DataNode; - public function DataNode(label:String) - { - _label = label; - } + public function DataNode(label:String) + { + _label = label; + } - public function get children():ArrayCollection { - return _children; - } + public function get children():ArrayCollection + { + return _children; + } - public function set children(value:ArrayCollection):void { - _children = value; - } + public function set children(value:ArrayCollection):void + { + _children = value; + } - public function get label():String { - return _label + (_isSelected ? " [SEL]" : "") + (_isPreviousSiblingRemoved ? " [PREV ITEM REMOVED]" : ""); - } + public function get label():String + { + return _label + (_isSelected ? " [SEL]" : "") + (_isPreviousSiblingRemoved ? " [PREV ITEM REMOVED]" : ""); + } - public function toString():String - { - return label; - } + public function toString():String + { + return label; + } - public function addChild(node:DataNode):void { - if(!_children) - _children = new ArrayCollection(); + public function addChild(node:DataNode):void + { + if(!_children) + _children = new ArrayCollection(); - _children.addItem(node); - } + _children.addItem(node); + node.parent = this; + } - public function set isSelected(value:Boolean):void { - _isSelected = value; - } + public function set isSelected(value:Boolean):void + { + _isSelected = value; + } - public function get isSelected():Boolean { - return _isSelected; - } + public function get isSelected():Boolean + { + return _isSelected; + } - public function clone():DataNode - { - var newNode:DataNode = new DataNode(_label); - for each(var childNode:DataNode in children) + public function clone():DataNode { - newNode.addChild(childNode.clone()); + var newNode:DataNode = new DataNode(_label); + for each(var childNode:DataNode in children) + { + newNode.addChild(childNode.clone()); + } + + return newNode; } - return newNode; - } + public function set isPreviousSiblingRemoved(value:Boolean):void + { + _isPreviousSiblingRemoved = value; + } + + public function get parent():DataNode + { + return _parent; + } - public function set isPreviousSiblingRemoved(value:Boolean):void { - _isPreviousSiblingRemoved = value; + public function set parent(value:DataNode):void + { + _parent = value; + } } } -} http://git-wip-us.apache.org/repos/asf/flex-sdk/blob/83e4a9b2/frameworks/tests/unitTests/mx/collections/HierarchicalCollectionViewTestUtils.as ---------------------------------------------------------------------- diff --git a/frameworks/tests/unitTests/mx/collections/HierarchicalCollectionViewTestUtils.as b/frameworks/tests/unitTests/mx/collections/HierarchicalCollectionViewTestUtils.as index bc1ca36..94b094b 100644 --- a/frameworks/tests/unitTests/mx/collections/HierarchicalCollectionViewTestUtils.as +++ b/frameworks/tests/unitTests/mx/collections/HierarchicalCollectionViewTestUtils.as @@ -19,6 +19,7 @@ package mx.collections { + import mx.collections.*; import mx.utils.StringUtil; import mx.collections.ArrayCollection; import mx.collections.CursorBookmark; http://git-wip-us.apache.org/repos/asf/flex-sdk/blob/83e4a9b2/frameworks/tests/unitTests/mx/collections/HierarchicalCollectionView_FLEX_34778_Tests.as ---------------------------------------------------------------------- diff --git a/frameworks/tests/unitTests/mx/collections/HierarchicalCollectionView_FLEX_34778_Tests.as b/frameworks/tests/unitTests/mx/collections/HierarchicalCollectionView_FLEX_34778_Tests.as index 9a85102..f436538 100644 --- a/frameworks/tests/unitTests/mx/collections/HierarchicalCollectionView_FLEX_34778_Tests.as +++ b/frameworks/tests/unitTests/mx/collections/HierarchicalCollectionView_FLEX_34778_Tests.as @@ -17,8 +17,7 @@ // //////////////////////////////////////////////////////////////////////////////// -package mx.collections -{ +package mx.collections { import mx.collections.ArrayCollection; import mx.collections.HierarchicalCollectionView; @@ -60,6 +59,18 @@ package mx.collections assertEquals(1, _sut.length); } + //this did NOT reproduce it, but it's good to test, because the code is in a different function + //(collectionChangeHandler) than the previous case (nestedCollectionChangeHandler). + [Test] + public function test_replacing_inaccessible_root_node():void + { + //when + _level0.setItemAt(new DataNode("Microsoft"), 0); + + //then + assertEquals(1, _sut.length); + } + private static function generateHierarchyViewWithClosedNodes():HierarchicalCollectionView { return _utils.generateHCV(_utils.generateHierarchySourceFromString(HIERARCHY_STRING)); http://git-wip-us.apache.org/repos/asf/flex-sdk/blob/83e4a9b2/frameworks/tests/unitTests/mx/collections/HierarchicalCollectionView_REPLACE_Tests.as ---------------------------------------------------------------------- diff --git a/frameworks/tests/unitTests/mx/collections/HierarchicalCollectionView_REPLACE_Tests.as b/frameworks/tests/unitTests/mx/collections/HierarchicalCollectionView_REPLACE_Tests.as new file mode 100644 index 0000000..c4d49a6 --- /dev/null +++ b/frameworks/tests/unitTests/mx/collections/HierarchicalCollectionView_REPLACE_Tests.as @@ -0,0 +1,325 @@ +//////////////////////////////////////////////////////////////////////////////// +// +// Licensed to the Apache Software Foundation (ASF) under one or more +// contributor license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright ownership. +// The ASF licenses this file to You under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +//////////////////////////////////////////////////////////////////////////////// + +package mx.collections { + import mx.collections.HierarchicalCollectionView; + import mx.collections.IList; + import mx.events.CollectionEvent; + import mx.events.CollectionEventKind; + import mx.utils.StringUtil; + + import org.flexunit.asserts.assertEquals; + import org.flexunit.asserts.assertFalse; + import org.flexunit.asserts.assertNotNull; + import org.flexunit.asserts.assertNull; + import org.flexunit.asserts.assertTrue; + + public class HierarchicalCollectionView_REPLACE_Tests { + + private static var _sut:HierarchicalCollectionView; + private static var _utils:HierarchicalCollectionViewTestUtils = new HierarchicalCollectionViewTestUtils(); + private var _root:IList; + private static var noItemsInHierarchy:int = NaN; + + [BeforeClass] + public static function setUpBeforeClass():void + { + const hierarchyLines:Array = HIERARCHY_STRING.split("\n"); + for(var i:int = 0; i < hierarchyLines.length; i++) + { + if(StringUtil.trim(hierarchyLines[i])) + noItemsInHierarchy++; + } + } + + [Before] + public function setUp():void + { + _sut = _utils.generateOpenHierarchyFromRootListWithAllNodesMethod(_utils.generateHierarchySourceFromString(HIERARCHY_STRING)); + _root = _utils.getRoot(_sut) as IList; + } + + [After] + public function tearDown():void + { + _sut = null; + _root = null; + } + + [Test] + public function test_make_sure_we_count_nodes_correctly():void + { + //when + var noChildren:int = 0; + for(var i:int = 0; i < _root.length; i++) + { + noChildren += countAllChildrenOf(_root.getItemAt(i) as DataNode); + } + + //then + assertEquals(noItemsInHierarchy, noChildren); + } + + [Test] + public function test_make_sure_isDescendantOf_works_correctly():void + { + //when + var region1:DataNode = _root.getItemAt(0) as DataNode; + var region2:DataNode = _root.getItemAt(1) as DataNode; + var city2:DataNode = region2.children.getItemAt(1) as DataNode; + var company2:DataNode = city2.children.getItemAt(1) as DataNode; + var department2:DataNode = company2.children.getItemAt(1) as DataNode; + + //then + assertTrue(isDescendantOf(department2, company2)); + assertTrue(isDescendantOf(department2, city2)); + assertTrue(isDescendantOf(department2, region2)); + assertFalse(isDescendantOf(department2, region1)); + } + + [Test] + public function test_replacing_a_childless_node_does_not_dispatch_REMOVED_collection_event_but_changes_parent_references():void + { + function onCollectionChanged(event:CollectionEvent):void + { + if(event.kind == CollectionEventKind.REMOVE) + removeEvent = event; + else if(event.kind == CollectionEventKind.REPLACE) + replaceEvent = event; + } + + var removeEvent:CollectionEvent = null; + var replaceEvent:CollectionEvent = null; + + //GIVEN + _sut.addEventListener(CollectionEvent.COLLECTION_CHANGE, onCollectionChanged); + var company2:DataNode = getDirectChildrenOf(1, 1).getItemAt(1) as DataNode; + var departmentsOfCompany2:IList = company2.children; + var firstDepartment:DataNode = departmentsOfCompany2.getItemAt(0) as DataNode; + + //WHEN + const newDepartment:DataNode = new DataNode("Region(2)->City(1)->Company(2)->DepartmentX"); + departmentsOfCompany2.setItemAt(newDepartment, 0); + + //THEN + assertNotNull(replaceEvent); + assertNull(removeEvent); //because the replaced node had no children + assertNull(_sut.getParentItem(firstDepartment)); + assertEquals(company2, _sut.getParentItem(newDepartment)); + assertEquals(noItemsInHierarchy, _sut.length); + } + + [Test] + public function test_replacing_a_node_with_children_dispatches_REMOVED_collection_event_and_changes_parent_references():void + { + function onCollectionChanged(event:CollectionEvent):void + { + if(event.kind == CollectionEventKind.REMOVE) + { + removeEvent = event; + + if(event.items && event.items.length == noChildrenOfSecondDepartment) + { + for(var i:int = 0; i < noChildrenOfSecondDepartment; i++) + { + if(event.items.indexOf(secondDepartment.children.getItemAt(i)) == -1) + REMOVEDEventHasChildrenOfSecondDepartment = false; + } + } + } + else if(event.kind == CollectionEventKind.REPLACE) + replaceEvent = event; + } + + var removeEvent:CollectionEvent = null; + var replaceEvent:CollectionEvent = null; + var REMOVEDEventHasChildrenOfSecondDepartment:Boolean = true; + + //GIVEN + _sut.addEventListener(CollectionEvent.COLLECTION_CHANGE, onCollectionChanged); + var company2:DataNode = getDirectChildrenOf(1, 1).getItemAt(1) as DataNode; + var departmentsOfCompany2:IList = company2.children; + var secondDepartment:DataNode = departmentsOfCompany2.getItemAt(1) as DataNode; + const noChildrenOfSecondDepartment:int = secondDepartment.children.length; + + //WHEN + const newDepartment:DataNode = new DataNode("Region(2)->City(1)->Company(2)->DepartmentX"); + departmentsOfCompany2.setItemAt(newDepartment, 1); + + //THEN + assertNotNull(replaceEvent); + assertNotNull(removeEvent); //because the replaced node had children + assertTrue(REMOVEDEventHasChildrenOfSecondDepartment); + assertEquals(-1, removeEvent.items.indexOf(secondDepartment)); + assertNull(_sut.getParentItem(secondDepartment)); + assertEquals(company2, _sut.getParentItem(newDepartment)); + assertEquals(noItemsInHierarchy - noChildrenOfSecondDepartment, _sut.length); + } + + [Test] + public function test_replacing_a_root_node_with_children_dispatches_REMOVED_collection_event_and_changes_parent_references():void + { + function onCollectionChanged(event:CollectionEvent):void + { + if(event.kind == CollectionEventKind.REMOVE) + { + removeEvent = event; + if(event.items && event.items.length == noChildrenOfSecondRegion) + { + for(var i:int = 0; i < noChildrenOfSecondRegion; i++) + { + if(!isDescendantOf(event.items[1] as DataNode, region2)) + REMOVEDEventHasAllChildrenOfSecondRegion = false; + } + } + } + else if(event.kind == CollectionEventKind.REPLACE) + replaceEvent = event; + } + + var removeEvent:CollectionEvent = null; + var replaceEvent:CollectionEvent = null; + var REMOVEDEventHasAllChildrenOfSecondRegion:Boolean = true; + + //GIVEN + _sut.addEventListener(CollectionEvent.COLLECTION_CHANGE, onCollectionChanged); + var region2:DataNode = _root.getItemAt(1) as DataNode; + var noChildrenOfSecondRegion:int = countAllChildrenOf(region2); + + //WHEN + const newRegion:DataNode = new DataNode("Region(X)"); + _root.setItemAt(newRegion, 1); + + //THEN + assertNotNull(replaceEvent); + assertNotNull(removeEvent); //because the replaced node had children + assertTrue(REMOVEDEventHasAllChildrenOfSecondRegion); + assertEquals(-1, removeEvent.items.indexOf(region2)); + assertNull(_sut.getParentItem(region2)); + assertEquals(null, _sut.getParentItem(newRegion)); + assertEquals(noItemsInHierarchy - noChildrenOfSecondRegion + 1, _sut.length); + } + + [Test] + public function test_replacing_inaccessible_node_does_not_dispatch_REMOVED_collection_event_nor_changes_parent_references():void + { + function onCollectionChanged(event:CollectionEvent):void + { + if(event.kind == CollectionEventKind.REMOVE) + removeEvent = event; + else if(event.kind == CollectionEventKind.REPLACE) + replaceEvent = event; + } + + var removeEvent:CollectionEvent = null; + var replaceEvent:CollectionEvent = null; + + //GIVEN + _sut.addEventListener(CollectionEvent.COLLECTION_CHANGE, onCollectionChanged); + var company2:DataNode = getDirectChildrenOf(1, 1).getItemAt(1) as DataNode; + var departmentsOfCompany2:IList = company2.children; + var firstDepartment:DataNode = departmentsOfCompany2.getItemAt(0) as DataNode; + + //WHEN + _sut.closeNode(_root.getItemAt(1)); //close second region + const newDepartment:DataNode = new DataNode("Region(2)->City(1)->Company(2)->DepartmentX"); + departmentsOfCompany2.setItemAt(newDepartment, 0); + + //THEN + assertNotNull(replaceEvent); + assertNull(removeEvent); //because the replaced node had no children + assertNull(_sut.getParentItem(firstDepartment)); + assertEquals(company2, _sut.getParentItem(newDepartment)); + + var secondRegion:DataNode = DataNode(_root.getItemAt(1)); + assertEquals(noItemsInHierarchy - countAllChildrenOf(secondRegion) + 1, _sut.length); + } + + + private function getDirectChildrenOf(...indexesOfSubsequentParents):IList + { + var currentLevel:IList = _root; + var i:int = -1; + while(currentLevel && ++i < indexesOfSubsequentParents.length) + { + var currentParent:DataNode = currentLevel.getItemAt(indexesOfSubsequentParents[i]) as DataNode; + currentLevel = currentParent ? currentParent.children : null; + } + + return currentLevel; + } + + private function isDescendantOf(node:DataNode, potentialAncestor:DataNode):Boolean + { + if(!potentialAncestor || !node) + return false; + + var currentParent:DataNode = node.parent; + while(currentParent && currentParent != potentialAncestor) + { + currentParent = currentParent.parent; + } + + return currentParent == potentialAncestor; + } + + private function countAllChildrenOf(parent:DataNode):int + { + if(!parent.children || !parent.children.length) + return 1; + else + { + var noChildren:int = 0; + for(var i:int = 0; i < parent.children.length; i++) + { + noChildren += countAllChildrenOf(parent.children.getItemAt(i) as DataNode); + } + + return noChildren + 1; + } + + return NaN; + } + + + private static const HIERARCHY_STRING:String = (City(0) + Region(2)->City(1) + Region(2)->City(1)->Company(1) + Region(2)->City(1)->Company(2) + Region(2)->City(1)->Company(2)->Department(1) + Region(2)->City(1)->Company(2)->Department(2) + Region(2)->City(1)->Company(2)->Department(2)->Employee(1) + Region(2)->City(1)->Company(2)->Department(2)->Employee(2) + Region(2)->City(1)->Company(2)->Department(2)->Employee(3) + Region(2)->City(1)->Company(2)->Department(3) + Region(2)->City(1)->Company(2)->Department(3)->Employee(1) + Region(2)->City(1)->Company(2)->Department(3)->Employee(2) + Region(2)->City(1)->Company(2)->Department(3)->Employee(3) + Region(2)->City(1)->Company(2)->Department(3)->Employee(4) + Region(2)->City(1)->Company(3) + Region(2)->City(1)->Company(3)->Department(1) + Region(2)->City(1)->Company(3)->Department(1)->Employee(1) + Region(2)->City(1)->Company(3)->Department(2) + ]]>). + toString(); + } +}