cerner / ccl-testing

A collection of maven plugins and their dependencies to perform CCL Unit tests and static analyses and to generate reports from the results.
Apache License 2.0
16 stars 11 forks source link

Issue deserializing F8 values on reply message #44

Closed jkuester closed 3 years ago

jkuester commented 4 years ago

I am seeing an issue where large F8 values are not being deserialized from the JSON reply record correctly. Basically, my script's reply message has an F8 field that gets filled out with the value 123541212.000000 (just a normal id value from a table). That value gets properly set in the JSON file and downloaded back to the machine running the tests. However, when the RecordWriter goes to create a Java object from the JSON String, the value that gets set in the Java object is slightly off (1.23541216E8).

This appears to be because the JSONObject utility uses the old NumberUtils library from the Apache commons-lang dependency to create the number from the JSON value and the version of NumberUtils in commons-lang has a defect where the value gets packed into a Float object instead of a Double and loses some precision. That defect was later fixed in commons-lang3, but AFIK nothing was patched back into commons-lang.

Am I doing something wrong here by using the F8 data type to store id values or is there some sort of workaround that I have not figured out?

feckertson commented 4 years ago

Can you provide an example project to illustrate?

Can you explain how this actually causes a problem? JSON data passed back to the maven plugin is only used for reporting purposes. It has no bearing on the pass/fail/error status of a test or on the alignment of code coverage data with source code.

jkuester commented 4 years ago

We ran into this issue while experimenting with some "off-label" usage of the j4ccl and j4ccl-ssh libraries. Basically we were trying to write the majority of our test code in Java, and then just have a small CCL test script that calls the actual CCL script we want to test. We would use the j4ccl libraries to compile and execute the test script and then return back the values we wanted to assert in the Java code.

I understand that is not really the intended usage of these libraries, so we are not looking for anyone to provide us a fix. However, it did seem worth raising an issue to make you all aware of the possible discrepancy in the reply data (even if it is just used for reporting purposes).

That being said, would you be open to us submitting a PR that would uplift j4ccl-ssh to use a newer JSON library (such as Gson)?

feckertson commented 4 years ago

PRs are welcomed.

My second question was mainly to gauge the actual need/urgency.

The issue is readily recreated with a unit test for j4ccl-ssh so there is no need for a project to illustrate the problem which appears to be an issue with net.sf.json.util.JSONTokener from net.sf.json-lib:json-lib.

        final String json = "{\"REPLY\":{\"AN_F8\":123541212.000000}}";
        final Structure structure = StructureBuilder.getBuilder().addF8("AN_F8").build();
        final Record record = mockRecord("REPLY", structure);
        RecordWriter.putFromJson(json, record);
        verify(record, times(1)).setF8("AN_F8", 123541212.000000);
        // or use RecordFactory to create an actual Record
        // and check whether the value returned matches the value set.

I gather (from conversations with others, elsewhere) that the goal is to drive things from cucumber, but I'm not sure that implies generating and compiling CCL code on the fly. Does that approach produce any reusable testing apparatus?

Brainstorming a little.... perhaps a BDD adapter could just create or add to a fixed CCL Unit test case. Then cucumber could simply run mvn test and report on the returned results, possibly parsing test-results.xml to provide finer detail. Such an adapter could possibly be made reusable.

Note that there have been some recent improvements to cclunit-framework allowing a CCL Unit test to mock database tables and seed the mocked tables with data allowing CCL Unit tests to be completely domain independent.

jkuester commented 4 years ago

Thank you for the feedback here! As you mentioned, our current goal is to leverage a Java BDD framework (Cucumber in this case) to handle our requirements and automated release testing for various CCL scripts that we own. In addition, our team does mainly Java development and we only occasionally write CCL scripts. The new scripts we do write are usually simple and follow established patterns (e.g. README scripts, custom combine scripts, wrappers for EJS calls, etc). This general lack of experience with CCL has made it difficult for our engineers to write quality tests in CCL (via the normal CCL Unit Test framework). So, we decided to try and use Java code to do the setup, assertions, and teardown steps and then only call into CCL to make the actual script call we want to test.

We have successfully used the j4ccl and j4ccl-ssh libraries to create request/reply records and execute scripts in test domains and even to compile short test scripts for wrapping other test calls or testing includes files. From a Java perspective, we found the j4ccl framework to be pretty flexible and intuitive once you understood the code flow :). So, hopefully that gives you a better idea of what we are up to here. I am not exactly sure what you mean by "reusable testing apparatus", but from our perspective, we are able to leverage all our existing Java utilities and tooling while relying on j4ccl to perform the script execution part of things.

A BDD adapter that would leverage the normal CCL unit test cases is an interesting idea, but would also require us to go back to writing more test code in CCL instead of Java (unless I am misunderstanding what you are suggesting).

feckertson commented 3 years ago

Fixed with PR #50.