Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Truncate namespace hierarchy #521

Open
paulens12 opened this issue Feb 12, 2020 · 11 comments
Open

Truncate namespace hierarchy #521

paulens12 opened this issue Feb 12, 2020 · 11 comments
Labels
Idea Just an idea... not yet accepted as a feature or enhancement

Comments

@paulens12
Copy link

Currently testcentric.exe shows the namespace of every test as a tree.

It often creates problems with long ("deep") namespaces and I'd like to propose a UI enhancement to make it easier to deal with those deep namespace structures.

I've created a short reproduction:

using NUnit.Framework;

namespace A.B.C.D.E
    {
    [TestFixture]
    public class F
        {
        [Test]
        public void Test1() { }

        [Test]
        public void Test2() { }
        }

    [TestFixture]
    public class G
        {
        [Test]
        public void Test1() { }

        [Test]
        public void Test2() { }
        }
    }


namespace A.B.C.D
    {
    [TestFixture]
    public class F
        {
        [Test]
        public void Test1() { }

        [Test]
        public void Test2() { }
        }

    [TestFixture]
    public class G
        {
        [Test]
        public void Test1() { }

        [Test]
        public void Test2() { }
        }
    }

image

As you can see, everything above D could be merged into a single node, which would reduce the oversized left margin and push the actual tests (the very thing we're interested in) towards the left edge so that more of their names are visible in a smaller window size. Generally, if a namespace has only one child item, they should be merged in the UI. For example, C has only one child item D so C and D should be merged. B and A also have only one child item each so all of them should be merged into a single node A.B.C.D.

@CharliePoole CharliePoole added the Idea Just an idea... not yet accepted as a feature or enhancement label Feb 12, 2020
@CharliePoole
Copy link
Contributor

I've marked this as an idea at this point, rather than an accepted feature. We'll need to work out some of the implications before settling on specific rules for combining and we'll need some implementation design. The following are just notes for consideration...

  1. The standard GUI nodes map one-to-one to the test nodes provided by the engine and ultimately by the framework. The code often needs to find the test node that generated a GUI node or the GUI node for a test node. The experimental GUI does not always have such a one-to-one mapping, so we might look at code there to get some ideas.

  2. "If a namespace has only one child item" may be too general a rule. What if that child item is a single fixture? What if that fixture has a single test? At a minimum, I think this should only apply to namespace nodes. I realize this may be what @paulens12 intended here, but I wanted to spell it out.

  3. Some namespaces have SetUpFixtures. As the tests run, it's possible for one of those setups to fail. Let's say we combine A.B.C.D and there is a SetUpFixture in namespaces A and D. If one of the two fails, then we'd like that to be clear.

  4. Right-clicking on any node allows you to bring up the properties for that node. We need to ensure that the correct information is displayed for combined nodes.

@CharliePoole
Copy link
Contributor

BTW, I'm currently focusing on back-end issues for the GUI, like getting .NET Core tests to run. Non-critical UI issues will probably not get much of my attention for the coming 1.3 release. Of course, once details are worked out, a PR would be welcome.

@immeraufdemhund
Copy link
Contributor

this reminds me of folder combine in vscode. where you can still drill into a specific folder if you need too, but by default the visuals of it shows as a single folder
image

@paulens12
Copy link
Author

I understand it's not top priority right now, I just wanted this to be out there. Here are some comments on the implementation:

  1. My guess is you should keep a reference on each UI node to the corresponding test node. Merged nodes could have a collection of references to all their namespaces if that's needed. But I haven't looked into the code so I don't really know anything specific.
  2. I don't have any strong preferences on this one. But I don't think having separate nodes for the namespace and class makes much sense. For example, if we removed A.B.C.D.F and A.B.C.D.G from the above example, I think it would make sense to merge everything into a single node A.B.C.D.E which would be represented as a test fixture, simply because A.B.C.D doesn't provide any more information (in its properties window or anywhere else) than A.B.C.D.E does. Unless we're talking about nested classes, in which case I'd have to think about it some more. Either way, applying this rule only to namespaces would make just as much sense.
  3. Well, let's say the setup fixture fails in A.B.C - in that case, the whole tree will be red. B will be red because all of its children are red, D will be red because its parent is red (am I right? D might also be gray, not so sure...). Anyway, the error message on the right should provide sufficient information about which part is failing because it includes the name of the failing part.
  4. I think you would either have to aggregate information from all the merged namespaces or add some kind of buttons to switch between them once you open the properties dialog.

Of course, this is just my opinion. I'm happy with whatever way this heads, this is just a small detail that I saw potential for improvement in.

@paulens12
Copy link
Author

Good point @immeraufdemhund - vscode's approach is a bit more complicated because it highlights the individual folders when hovering them with mouse cursor. I feel like we should come up with a simpler solution here because there seem to be few people supporting this project and there's already a lot on the plate. Perhaps somehow joining the properties windows would be easier.

@CharliePoole
Copy link
Contributor

@immeraufdemhund Worth considering... we could label differently depending on whether the user expanded or not.

@CharliePoole
Copy link
Contributor

@paulens12 Considering all the possibilities, the best way to proceed is probably to do something small and simple first, then consider whether to do more. For example...

  1. Combine strings of namespaces provided none of them has a SetUpFixture
  2. Combine strings where the last namespace has a setUpFixture.
  3. Combine strings where there is only one setupfixture in the run.
  4. Combine strings with multiple setup fixtures.

The above is just one possible order of implementation, of course. The first is trivial. The second, however, already introduces complexity in terms of the update of nodes as the tests complete. Since tests complete bottom up, a setup fixture failure would post an error message but the higher level namespaces would override it with a "child error" message unless we prevented that from happening. Of course, we would want to prevent that and we could, but it's just a sample of how baked-in the one-to-one assumption is at this point!

That's less so in the experimental GUI, which is my eventual target for a major release, so some of this may need to wait until that point. The GUI portion of the standard GUI is very much still like the V2 GUI.

@paulens12 Are you of a mind to possibly take on the simplest step?

@paulens12
Copy link
Author

I'll see if I can do it, but can't promise anything as I've been quite busy lately and that doesn't seem to be going away soon (working on my bachelor's thesis etc.) - I always find time to read comments, even if it's only on my phone, but not so much "me and my computer" time outside work these days.

@paulens12
Copy link
Author

The question is if it's worth the effort to implement this the proper way for the "old" GUI or if it's better to just wait for the experimental GUI to mature. Right now, the old one seems more stable and finished (i.e. no "not implemented" menu items or similar) and I don't know the projected timeframe for a stable new GUI release. But if it's, let's say, less than half a year, maybe it's worth focusing on the new GUI and limiting old GUI updates to bug fixes only?

@CharliePoole
Copy link
Contributor

Personally, I'm a big fan of small steps. "Doing it right" sometimes means adding lots of features at once. But the small steps have to meet a few criteria...

  • Each step of value on its own.
  • Each step building toward an eventual goal, but not requiring us to take further steps if we change our collective mind.
  • No step that will later be removed in a way that causes a breaking change, however the particular project defines a breaking change. We have a pretty good statement of what we will and won't do between major releases in VERSIONING.md in the project root.

So, if you have time, I'd say just combine runs of namespaces that do not contain any setup fixtures. That's probably the most common situation for typical users anyway. Also, we would want a setting for the tree that says something like "Combine namespaces where possible."

I think the experimental GUI is 6 to 12 months away as far as becoming the main GUI.

@CharliePoole
Copy link
Contributor

BTW... focusing on bugs only in the standard GUI is a step I expect to take, but not right away. I'm very close to getting .NET Core tests to run from the GUI in combination with .NET Framework tests (in separate processes, of course) and that will make us (I think) the only runner that is able to do so. I'd like to get that out in the standard GUI before switching focus.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Idea Just an idea... not yet accepted as a feature or enhancement
Projects
None yet
Development

No branches or pull requests

3 participants