VI Analyzer Enthusiasts Discussions
Proposal - Test Improvements and Removals in LabVIEW 2018
I am considering making the following changes to the VI Analyzer Toolkit test offerings with the LabVIEW 2018 release. Please let me know what you think about these potential changes:
- Remove 'Node Density' test - We had high hopes for this test, but I have yet to hear anyone claim that it is helping them achieve cleaner code. More often than not, I hear complaints about users being unable to discern what exactly the test is detecting and how to correct it. So I'd like to remove the test if nobody finds it useful in its current form.
- Improve 'Wire Bends' test - Add configuration options to the Wire Bends test to allow for wires with multiple sinks. Here's a mockup of what the configuration options would look like:
- Improve 'Wire Crossings' test - Same as before, but with the Wire Crossings test.
- Remove 'Typedef Cluster Constants' test - This test was useful back when LabVIEW had a bug with data being lost in typedef instances of clusters when the cluster data type changed. This bug was largely fixed with the 'Review and Update from Type Def" feature added a few releases ago, so I don't think we need this test anymore.
- Include vi.lib, instr.lib, and user.lib in 'SubVI and TypeDef Locations' test by default - The list of folders in the SubVI and TypeDef Locations test should include vi.lib/instr.lib/user.lib by default (like the 'Fan Out' and 'Modularity Index' tests do), with the option to remove them if the user so desires. The test will also skip running if only the vi.lib/user.lib/instr.lib folders are listed, just like right now it will skip running if no folders are listed. You have to explicitly add at least one of your own folders to the list before the test will execute on your VIs.
- Remove 'Connected Pane Terminals' test - I have never had an experience where this test revealed a bug in my code. Instead, it always returns failures for terminals in templates or in class override VIs, which are perfectly fine to be unwired.
- Change default configuration of 'Error Style' test - In keeping with my Brainless LabVIEW declarations, I would like to deselect the 'Error Input Wired to Case Structure Selector' configuration option in the default test configuration. You would still be able to select this checkbox if you wish, and existing .cfg files would be unaffected.
- Remove default patterns from 'Terminal Connector Type' and 'Terminal Positions' tests - I haven't found these tests useful with their default configuration options. They always return false failures, and I've never had them reveal for me a mistake that I need to correct. If I have the lists be empty by default, then the tests can still be used for detection of certain patterns, but will no longer return false failures when a default configuration is run. Existing .cfg files would be unaffected by this change.
I anticipate the following upgrade issues based on these proposed changes:
- Removed tests - You'll get a popup when you load a .cfg that contains removed test configurations, saying that something about your configuration changed since the last save.
- New config parameters - This refers to the Wire Bends and Wire Crossings tests. Your existing configuration for these tests will be wiped out, and the new options will take effect.
- Same config parameters with new defaults - No user-visible effect, as the .cfg file for tests will preserve the previously-saved configuration options.
Let me know what y'all think of these proposed changes, and if any of them would be showstoppers for your development processes.
Overall looks good, there's no change there that I would complain about. That being said I do have some comments.
I realize the ignore tests functionality added an undesired performance hit when applied to all tests. But has NI considered only adding it to a subset of tests?
I've noticed that for some of these tests you've mentioned I would still choose to test for them, but I've been using the ignore tests by adding a BD comment. Like for instance the Error Style I still check for and I still want to review places that I'm not using an error structure, but in cases where when I wrote the VI I was already thinking about error handling, I would just add the ignore test comment. For this I'd prefer if the Error Style wasn't removed completely. But if NI isn't going to add the ignore functionality due to performance issues, I guess I don't have a strong enough feeling to care if it is on or off by default. The Error Style test has helped me find some bugs in the past. Since it will force me to think about error handling a bit where maybe I wasn't before.
Darren wrote:
Hooovahh wrote:For this I'd prefer if the Error Style wasn't removed completely.
I wasn't planning on removing the test, just changing the default configuration to have the 'error case around diagram' checkbox deselected.
Oh right sorry, changed my thought half way through. Great to hear that the ignore test functionality is still being looked into, hope we can see it be implemented in some official capacity at some point. Until then the solution using the code I linked to sure does come in handy, and I've been using it on about 10 different tests pretty successfully. I never timed it, but performance still seems acceptable for me.
