NUnit tree: add option to show/hide namespace nodes; add folding of namespace nodes #1153
Conversation
|
FYI, I'm having trouble with this one, in part because there are two different changes and it's hard to pull it apart that way, but mostly because it's very difficult to visualize what it looks like and how it works. I think I'll pull it down and run it. |
|
I built and ran it and now I see what you're doing. There are a few changes I'd like to see, which I'll get into the review after lunch. But here's are a few points in case you are up and about at this time and want something to do... I think the UI for showing namespace as an additional choice under NUnitTree works. If we succeed in getting rid of the other two strategies, then we can put the option on the main menu as if it were a new strategy. But for the submenu, I'd like to see two radio buttons... Show Namespaces and Hide Namespaces. The verbs make it clearer I think. For the folding operation, it all looks good except for the XML display. I think it should show the higher level rather than the deepest level. In this case, showing full info seems to trump consistency. |
|
Thanks for your feedback - I'll adapt these parts 👍
But I'm already heading to bed now, so I'll start tomorrow :-) |
|
I couldn't figure out where to add this in the code, so I'll just make a general comment. I think that the assignment of the deepest node to a folded tree node works in all situations except one: when we display the XML. In that case alone, I'd like to see the XML from the topmost node down. I realize that you have it set up to always use the same tree node for all purposes and this is a possibly messy complication. So if you prefer we can merge it the way it is and revisit at a later time. |
| { | ||
| return testNode.Name; | ||
| } | ||
|
|
There was a problem hiding this comment.
The name of this method is slightly confusing. Up till now, each tree node representeed only one test node. But now, with folding, it can map to multiple namespaces. So... each test node has a name and the tree node may have a combined name. It would be great if this method and the next one could reflect that in some way. GetTreeNodeName and GetTreeNodeDisplayName?
| @@ -18,6 +19,8 @@ namespace TestCentric.Gui.Presenters | |||
| /// </summary> | |||
| public class NUnitTreeDisplayStrategy : DisplayStrategy | |||
| { | |||
| private IDictionary<TestNode, string> _foldedNodeNames = new Dictionary<TestNode, string>(); | |||
|
|
|||
There was a problem hiding this comment.
Did you examine whether the base class dictionary, _nodeIndex, which is used for other purposes, could be expanded to serve the purpose of _foldedNodeNames? If not, we could look at it at a later time to decide whether it would simplify things.
| this.nunitTreeShowNamespaceMenuItem.Size = new System.Drawing.Size(198, 22); | ||
| this.nunitTreeShowNamespaceMenuItem.Tag = "NUNIT_TREE_SHOW_NAMESPACE"; | ||
| this.nunitTreeShowNamespaceMenuItem.Text = "Namespace"; | ||
|
|
There was a problem hiding this comment.
Let's make the text "Show Namespace" so it's clear what it does. I'd actually prefer to see both Show Namespace and Hide Namespace exclusive radio buttons to make it completely clear, but I think we will redo this whole menu thing at least one more time so it could wait.
|
Yes, right! This single menu item looks a little lost - using two menu items is much better. |
|
I assign the top most TestNode of all folded Namespace nodes to the TreeNode, now. I'm really fine with this decision! |
|
And I adapted those method names as you suggested: |
|
I also had a quick look to the I will start the commit now, and if everything is ok and succeeds, it's ready to merge from my point of view. |
…ssign top most TestNode of folded namespaces to TreeNode
This PR solves issue #1151 and #1152 which both are dealing with the NUnit tree:
From user point of view there's a new submenu item which controls if the namespace nodes are shown or hidden in general.
This menu item is only enabled for the NUnit tree, but disabled if FixtureList or TestList mode is active.
Here's a screenshot: Namespaces are shown on left side, on right side they are hidden
In addition the namespaces are getting folded (if applicable). Here's an example:
From a technical point of view, several aspects had to be taken into account.
The core of this issue is implemented in the class NUnitTreeDisplayStrategy: it's taking care about omitting namespaces and folding of namespaces. I decided not to change the base method
MakeTreeNodewhich is used by all Strategies, but to add the implementation in the NUnitTreeDisplayStrategy class itself. So it's kept independent, avoiding any unintended side effects. However the recursive call to create the nodes for the children are done here, now.One important line of code is how to detect a namespace reliable - thanks to your hint about SetUpFixtures, I consider this case as well.
When folding the namespaces, I decided to assign the 'deepest' of the folded TestNodes to the resulting TreeNode. All other folded TestNodes will not have a corresponding TreeNode. Some special handling was required for the TreeNode name of folded namespaces. Thanks to the new feature 'Show test duration', the tree node name must be adapted whenever 'Show test duration' is switched on/off and therefore it's not kept untouched after tree node creation. I had the impression that it's difficult to determine the folded name afterwards, so I decided to keep a list of all folded names in the NUnitTreeDisplayStrategy. That list can be used, whenever a tree node name is requested.
VisualState/Settings:
I implemented the ShowNamespace option identically to the Strategy or Grouping option: that means that it's stored in the settings and in the VisualState file. The VisualState file ensures that the same visual tree representation is restored whenever a test project is loaded. And the setting option ensures that the last selected option is applied when opening a project without VisualState file.
And I extended some tests for these new use cases.