Untested MDSplus builds

[smithsp]

Affiliation
General Atomics

Version(s) Affected
All recent versions

Platform
RHEL 8

Describe the bug
There have been various issues.

To Reproduce
Ask @ModestMC for tests

Expected behavior
Be able to connect to a server running an older version of MDSplus via IDL, python, and matlab.

Additional context
We are currently building a new cluster and production server at DIII-D. We need to have a working version of MDSplus to move forward. We are willing to provide access to our systems so that the MDSplus builds can be tested in our environment according to our requirements. We are also willing to provide requirements to improve the automated testing to avoid these problems.

Mention @sflanagan @ModestMC

[mwinkel-dev]

Hi @smithsp -- You are correct that the MDSplus test suite needs improvements. (For example, I cannot find any IDL tests in the test suite.)

The recent failures of my first two fixes for Issue #2625 do indeed underscore the need for better tests. This is not an excuse but rather an explanation of what went awry. Simply put, there were these factors: new developer, no IDL test infrastructure, missing context in the bug report, and pressure to fix ASAP.

• Fix 1. I was tasked with fixing Issue 2625 ASAP; Sean had suggested starting the connection / socket IDs at 1 (instead of zero). As a new member of the MDSplus software team, it never occurred to me that DIII-D had MDSplus servers that could not be upgraded. And thus when GA tested this succinct fix to the C code, the fix broke because new software was contacting a sever with old software. As you noted, this highlights the need for cross-version testing with multiple servers. This feature will have to be added to the test suite.

• Fix 2. The second approach was to find an IDL-only work around to IDL's poorly implemented keyword_set() function, and that also would not require extraneous connections (to preserve the best possible performance). This fix was riskier than Fix 1 because it involved more code changes, plus I was not confident it would work for all mdsvalue scenarios. Nonetheless, the fix did work for the scenarios described in the initial report for Issue 2625. However, because I am new to the team, I didn't realize that I had to test the full IDL API. After GA reported problems with Fix 2, other portions of the IDL API were investigated and uncovered some places that I was unaware needed to be included in Fix 2. Unfortunately, the investigation also revealed some scenarios with mdsvalue that could not be fixed with this technique.

• Fix 3. Sean also suggested the the fix of last resort should be to open an extraneous socket (i.e., take a minor hit on performance). Doing so bypasses the limitation with IDL's keyword_set() function. That fix has been completed and appears to work. However, as there is no full test of the IDL API, there remains a risk (albeit low) that there might be issues with this fix too. A PR for this fix will be submitted tonight around 11:20 p.m. EDT. So after the usual review process (and manual testing), will be merged to alpha and made available to GA.

• Fix 4. It is possible to write a new function, mds_keyword_set() to replace IDL's poorly implemented keyword_set() function. Initial testing of this approach works well. The benefit is it eliminates the extraneous connection of Fix 3. The downside is that it has a somewhat larger footprint (however, considerably less than Fix 2).

Summary is that we appreciate the bug reports, feedback and offers to collaborate on improving the test suite.

Mention: @sflanagan @ModestMC @WhoBrokeTheBuild

[joshStillerman]

@smithsp Thank you for your feedback and help on this issue, and on testing in general. As you can see above, we have been diligently working to solve the problem that @sflanagan found. We are are working closely with @sflanagan and @ModestMC, we are finding this very useful and fruitful.

Please keep the questions, bugs and feedback flowing.
_josh