Use of Settings in TestCentric

[CharliePoole]

This issue is a reboot of issue #1214, which it replaces. Much of the discussion in that issue may still be useful as a reference but a lot of things have changed...

1. We no longer have our own engine but use NUnit's engine and have to deal with NUnit package settings as well as some settings of our own.
2. A new type-safe form of package setting has been introduced and each setting defines it's own default values.

The word "setting" is overloaded. This issue considers three types of settings and we will use different names for them in the discussion.

1. Application Settings are persistent settings saved by an application for regular use.
2. Project Settings are settings used in a particular TestCentric project.
3. Package Settings are an NUnit concept and refer to settings incorporated in a TestPackage.

Each of these settings types will be explained further, followed by a checklist of actions needed to complete this design issue.

TestCentric Application Settings

TestCentric Application Settings were originally saved in the TestCentricSettings.xml file but are now saved in a user.config file using Windows class UserApplicationBase. While all settings changes are triggered by user action, only a small number are set explicitly through our Settings dialog. Many other settings are saved "behind the scenes" by TestCentric when the user makes a change. For example, whenever the main TestCentric window moved or resized, the settings controlling window positioning are changed so that the Window will appear in the same way the next time the application is opened.

Settings Dialog

This multi-paned dialog is used for changing and saving a number of application Settings explicitly. Because many of these are also saved as individual project settings (see next section), they are only useful when creating a new project.

TestCentric Project Settings

When a TestCentric project is saved two files are created. A .VisualState.xml file contains items needed to restore the visual appearance of the GUI when the project is reopened. A .tcproj file is used to save non-visual settings so that everything works the same as when the project was last used.

NUnit Package Settings

In the V3 NUnit engine package settings were simply objects saved as the value of a name in a Dictionary<string, object>. That meant that each use of a setting required a cast at the call site and a generic method was used for that purpose, with the specific type indicated by a default value parameter. The specified default could be different for different calls, which sometimes led to errors.

The V4 engine, which we use, encapsulates each setting as an object, with the required Type and default value included. When creating a TestPackage for use with the NUnit engine, we generally only specify a particular setting if it differs from the default. Our goal is to rely as much as possible on NUnit's built-in defaults, reducing the values that are saved for the application as a whole and for individual projects.

---

Decisions Needed

The following list -- which may be expanded as we proceed -- includes the specific decisions that need to be made to complete this issue. Individual items will be checked off once a decision is made and any required implementation issues created. The current issue will not be used for any implementation details.

• Which settings should we continue to support in the Settings Dialog?
  DECISION: We'll radically reduce the scope of the Settings Dialog as per the discussion in this issue.
• Can we eliminate all settings used as "user defaults" for new projects?
  DECISION: Not all but most can be eliminated.
• For any settings to be removed from the dialog, will they need to be added elsewhere in the GUI? Where?
  STATUS: let's revisit this one after Reduce the scope of the Settings Dialog #1413 is completed.
• Should the .VisualState.xml file and .tcproj file be combined?
  DECISION: We'll keep them separate.
• If they are not combined, which specific settings should be saved in each file?
  DECISION: No action needed.
• Should TestCentricProject continue to derive from TestPackage or should we use composition instead?
  DECISION: We will switch to use of composition.

Issues created to carry out our decisions

• Reduce the scope of the Settings Dialog #1413
• Modify TestCentricProject to aggregate the top-level TestPackage #1414

[rowo360]

Thanks for the detailed summary. I feel like my own thoughts are not yet completely sorted out, so here's the current state:

Which settings should we continue to support in the Settings Dialog?
That seems like the easiest question to me. Because I think we can keep all the current settings in the Settings dialog. Please note, that I consider the settings (MaxAgent, ShadowCopy and PrincipalPolicy) already as removed from the settings dialog - even though my commit was only 2 days ago. Some special remarks are required for the two settings InitialDisplayOnLoad (Collapse/Expand/...) and ShowCheckBox which will I do separately - because it applies to the question "user default for new projects?" But besides these two all settings define the general visualization or behavior of the application for any project. So, the settings dialog is a good location for the configuration.

Can we eliminate all settings used as "user defaults" for new projects?
I really liked how we managed to simplify the use cases for the MaxAgent, ShadowCopy and PrincipalPolicy settings. Just by considering the use cases and finally decide that there's no need to maintain user defined default values for these settings when creating new projects.
Besides these three settings there are additional user defined default values for the tree visualization. For example: the two settings mentioned already above (InitialTreeLoad and ShowCheckBox in the Settings Dialog), but also the TreeDisplayFormat, GroupBy and ShowNamespace settings. These settings are not configured in the settings dialog, but set transparent whenever the user changes the values for the current project (using toolbar or menu items). So, currently default values for new projects are getting changed, while configuring the current project. I find this approach a little too complex and would like to simplify it. Here are two different proposals:

1. There are no user defined default values for the visualization. Instead we define some default values for new projects and the user has to deal with it. For example, no checkboxes are shown for new projects and NameSpaces are always shown. We would remove all these settings in the settings dialog and also remove the transparent settings. Well, that's certainly a simple approach, but I don't think it will appeal to all users. Some users might prefer checkboxes so much, that they want to display them always.
2. With this proposal, the user-defined default values are retained. But I suggest to remove the transparent approach of setting default values. Instead, I would like to show+configure the visualization default values explicitly. For example, the setting dialog might contain a "Initial tree display for new project" section. This section contains all these settings: 'ShowCheckbox", "DisplayFormat", "GroupBy" and "ShowNamespace", but the description makes it clear that it's a configuration of default values for new projects and is not applied to the current project right away. If the user wants to change the visualization of the current project, he can use the configuration via context menu/toolbar buttons. (The values for the current project are stored in the VisualState file - see next question). Overall, the default values for new projects and the values for the current project are clearly separated from each other. That seems to be a step forward.

Should the .VisualState.xml file and .tcproj file be combined?
That seems to be a good question, because it has sparked a new line of thought for me: I like the approach of the VisualState file, that it's saved transparent in the background (without any explicit user action) and whenever I reload the test assembly I can continue my tests with the same visualization just as I left it. And when I think about it, I believe that users expect the same thing from the engine settings (ShadowCopy, MaxAgent, RunAsX86, TestParameters...). So, some transparent saving of the engine settings, so that he continue with his tests on reload with the same set of engine settings as before.
I'm not sure whether it would be better to have one combined file or two separate files. Similar to the VisualState file, this new file(s) would be saved next to the test assembly. If it's a combined file, we can discuss if it's beneficial to store it XML format or TestPackage format or something different.

I try to extend the previous considerations which dealt with one single test assembly, to multiple test assemblies now. When working with multiple test assemblies we cannot rely on the transparent saving anymore, but instead the user has to explicitly save his project into a dedicated location. At this point, I'm not quite sure how best to proceed. If we decide to use one combined file (for engine settings+subpackages+visualization) then the project file would contain all information and would be independent of any of those transparent files mentioned above. Even if we decide to have two separate files, we still could save and use them independently. However, a different option would be if we use two separate files, but don't save the project visualization separately but use the existing Visualization files which are located next to the test assemblies - that's how it's done today.

Hmm, one combined file sounds beneficial - transparently saved for single assemblies and explicitly saved for multiple assemblies (completely independent from each other). I could share/check-in such a file to a co-worker and he can continue with the same settings/visualization.

[CharliePoole]

Settings to support in the Settings Dialog...

@rowo360 I'm going to try to separate out the questions we need to resolve so we are discussing one at a time in a given string of comments. My hope is to resolve the easiest ones first, so I'm starting with the one you indicated as easiest. :-)

That seems like the easiest question to me. Because I think we can keep all the current settings in the Settings dialog. Please note, that I consider the settings (MaxAgent, ShadowCopy and PrincipalPolicy) already as removed from the settings dialog - even though my commit was only 2 days ago. Some special remarks are required for the two settings InitialDisplayOnLoad (Collapse/Expand/...) and ShowCheckBox which will I do separately - because it applies to the question "user default for new projects?" But besides these two all settings define the general visualization or behavior of the application for any project. So, the settings dialog is a good location for the configuration.

I agree that this is relatively easy and that we should drop those three settings you just dropped.

So, looking at the dialog, there are five settings pages....

1. General This selects the Full vs the Mini Gui, the visibility of the status bar, the number of recent files to list whether to check that files exist before listing them and whether to load the most recent file at startup. It seems to me that all of these can be eliminated. The two most important ones are already in the View menu and the others can just be given defaults when a new project is created. Note that the --noload command-line option is available for use in shortcuts as are the --mini-gui and --full-gui options.
2. TreeDisplay This has three settings. The first, initial display on load, I think we can drop, using the existing Auto setting, which works well. I think Tree images and Checkbox would be OK to keep but only if we are providing for user-defined display options.
3. Assembly Load Settings - the three settings you already removed, so we can (or have) dropped this.
4. Automatic Reload This has only one functional setting re-run last tests run when reloading may be a setting to keep - if we keep any at all.
5. Project Editor Since we don't support editing of .nunit files any longer - and I did not plan to bring it back - I think this page can be dropped.

So, on the surface we have almost opposite conclusions. But it's possible we have different definitions. By Settings Dialog I mean the dialog that maintains persistent default settings for the application. I think we can reduce those settings significantly, possibly three or four settings only. If we are willing to give up user-specified defaults for new projects entirely, then the entire Settings Dialog could be eliminated along with all it's tests!

Of course, the user will still need to set many of these for individual projects and the UI may look like - even use some of the same code as - the current Settings Dialog. Or it may not. Does that put us more back on the same page? Ha ha!

I'll come back separately for each of the other points.

Charlie

[CharliePoole]

About "User defaults for new projects...

I like your second approach. A minimalist set of user options, consisting of those choices that a large number of users would like to standardize in their usage. Checkboxes are a great example. I have observed that some users want to use them all the time, while others prefer to avoid them. We should not use this for settings that tend to vary by project rather than according to the mindset of the user.

I suspect that we may have different ideas on how many things need to be made available this way. I can imagine not supporting it at all but for a minimal group, I imagine a single page with three or four settings. However, we can hammer the details out on the issue we create for the purpose.

Once we reach agreement on this decision and the previous one, I can create the implementation issues for them.

[CharliePoole]

About possibly combining the .tcproj and .VisualState.xml files...

I once talked about doing a TC console runner, but no longer think I'll be writing one. Nevertheless, it might be an interesting thought exeriment to imagine one. If we had such a runner, it would need the ability to open and handle .tcproj files but any GUI elements would not be necessary. Of course, it could ignore such elements but separate files makes more sense in that imagined situation.

Other reasons I favor continued separation...

• Both files already handle multiple assemblies as well as single assemblies. If we are now saving separate visual states for each assembly in a project, I think that's probably an error, but we can discuss it as part of implementation.
• Designing and implementing a single file is work we would otherwise not need to do.

WRT the final paragraph of your comment, I'm not sure I understand what you mean by saving transparently for single assemblies but explicitly for multiple assemblies. I think a project is a project, whether it contains one assembly or many.

[CharliePoole]

Another word on "new project defaults"...

Should we consider not having them at all but instead to provide the user with a wizard that leads them step by step through creating a project? Possibly in version 2.1?

[CharliePoole]

About inheritance vs composition in the TestCentricProject class...

I added this question because I can see that the decision to derive TestCentricProject from TestPackage, which I made as a matter of expedience, was not the best design choice. It's not correct that a project ISA package. The use of inheritance just made it easier to borrow code from TestPackage, which is usually not a good reason for inhiritance.

Roald, you pointed out and fixed some issues that made me aware of this problem. I now think it would be better for the TestCentricProject class to hold a TestPackage and provide the package with settings when necessary. When creating a new project, the project should have properties for those values it will use to initialize the TestPackage. When loading a project, it would load the contained TestPackage and set it's own properties from certain of the package settings found there.

Precisely which properties should be defined would be a question for the implementation. If we agree on this design point, I'll work on the implementation myself.

[rowo360]

Settings to support in the Settings Dialog...
I realize that I haven't thought as radically as you have, but rather in simpler terms. But with your explanation, I agree with each of your suggestions for the individual settings. Overall, this will reduce the settings dialog essentially. However, I don't propose to drop the dialog completely, but keep it for the few remaining settings.
I also imagine that the 'user default settings' (see next section) will be placed in a settings dialog.

[rowo360]

About "User defaults for new projects...
I'm glad you like my idea. 😄 Above all, I see this as a simplification: currently, the “Show Checkbox” value can be set in the Settings dialog or in the context menu, and we always have to keep this synchronized. This has become somewhat too complex in the implementation. By defining and describing in the UI that the configuration in the settings dialog is only the default value for new projects, I expect that it's better separated.
And of course, we need to discuss and agree upon if there will be any further user defined default values for the visualization. Or if we can define fixed default values which fit for (almost) all users.

[rowo360]

About possibly combining the .tcproj and .VisualState.xml files...
I still find this question difficult to answer, as I don't yet have the overview about all the implications. But I can see the effort involved in implementing such a combined file when we already have the separate files.
But I notice that my thoughts are still one step ahead. I find it a bit strange that when using multiple test assemblies, we store the complete VisualState in the VisualState.xml from the first test assembly. It's good that we have a complete VisualState, but I prefer to save it separately from the individual VisualState.xml which are located next to the test assemblies. So, an idea might be to save the complete VisualState in a separate file next to the tcproj file or well, if we decide to combine them, it's only one single file.
In case there are separate files the file system looks like this:
MyProject.tcproj
MyProject.VisualState.xml

What do you think about my idea to also store the engine settings transparently for individual test assemblies?
Like we did for the VisualState.xml already nowadays. If we have a combined file and we continue to save it transparently in the background at the same point in time, then the engine settings will be included automatically. I think that would be beneficial for the users because they don't need to configure the engine settings over and over again (for example TestParameters) and we are not forcing them to explicitly create a tcproj file.
But even if we decide to keep the files separately, we could start to create a tcproj file transparently to restore the settings on reload. The result would be the same for the user. So, it will look in the file system like this:
MyLibrary.Test.dll
MyLibrary.Test.dll.VisualState.xml
MyLibrary.Test.dll.TestCentric.tcproj or MyLibrary.Test.dll.tcproj or...

Your example with the TC console runner clearly reveals that such a runner won't require any visualstate settings. If I imagine it even further that this console runner can create/modify tcproj files, he needs to ignore the visualstate aspects or write through all unknown aspects. So, separate files for these to separate concerns sounds to be the better option. The only concern I can think of is that these two files should always match. So they should not diverge (one file has 2 assemblies, the other 3 assemblies). But, I believe the tcproj file will always be the leading part - so this concern can be easily solved.

[rowo360]

About inheritance vs composition in the TestCentricProject class...
I notice myself that I tend to derive a class too quickly, although I think it is generally recommended to prefer aggregation over derivation. So, your idea is fine. I also understand the example with the own properties while creating/loading a project. Do you have an example for such a property? Then I can imagine it even better. 😄

[CharliePoole]

Settings to support in the Settings Dialog
I think we have a decision for this item, i.e. "radically reduce the settings dialog," which will in future serve only two purposes...

1. Providing a small number of default values for new projects. Example: maybe user can specify strategy but not grouping or other subordinate settings.
2. Providing a small number of Gui-related user preferences where we believe that the typical user preference would not be based on the project. Example: maybe I always prefer a particular set of icons.

I say "maybe" at this point to indicate that we'll make the final decision in an implementation issue and PR.

See changes I made to the checkbox list of decisions and issue #1413

[CharliePoole]

** User Defaults for New Projects

I'm marking this decided as well. Issue #1413 should be able to handle the details.

[CharliePoole]

@rowo360 You asked...

What do you think about my idea to also store the engine settings transparently for individual test assemblies?

Did you see my earlier comment?

WRT the final paragraph of your comment, I'm not sure I understand what you mean by saving transparently for single assemblies but explicitly for multiple assemblies. I think a project is a project, whether it contains one assembly or many.

I'll expand... I was assuming that any individual assembly opened became a project. Of course, the user could say "No" when asked if it should be saved. If they then re-opened the same assembly, they would get a completely new project with no Visual State saved. So... what would you want to have happen in this context?

Also, I realize I'm not clear as to what you mean by "transparently" in this context. Is it saving without asking? Currently, I guess we always save visual state without asking because we need it internally but we ask before saving a .tcproj.