added test cases

[uli-f]

I added a variety of test cases from https://github.com/IUPAC-InChI/RInChI/ in PR #10.

@ntk73 Please merge PR #3 first, then PR #10.

[ntk73]

@uli-f , I suppose you have seen that PR #10 was closed and we have created a new clean PR #16. You have added several new test cases and corresponding recourse files. This is great.

I have one small remark concerning the small changes of previous test cases: you have added several checks for null pointer before junit assert test. I do not think this check is needed because if something goes wrong the junit test will fail with a null pointer exception for instance - and this is the main goal of the junit test to catch what is wrong. Actually, with a such null pointer check, it is possible some problems to stay hidden because assert is not performed.

[uli-f]

@uli-f , I suppose you have seen that PR #10 was closed and we have created a new clean PR #16. You have added several new test cases and corresponding recourse files. This is great.

Thank you. I am all for putting in as many tests as possible!

I have one small remark concerning the small changes of previous test cases: you have added several checks for null pointer before junit assert test. I do not think this check is needed because if something goes wrong the junit test will fail with a null pointer exception for instance - and this is the main goal of the junit test to catch what is wrong. Actually, with a such null pointer check, it is possible some problems to stay hidden because assert is not performed.

I assume you refer to the code in JnaRinchiTest::genericExampleTest(String,String):

if (rfi.getAuxInfo() != null) {
      assertEquals(rfi.getAuxInfo(), rinchiOut.getAuxInfo(), "RAuxInfo for " + reactionFile);
}

I did this because some of the rinchi files (e.g., two_reactants_no_products.rxn.rinchi_strings.txt) I added did not have all the information/fields (RInChI=, RAuxInfo=, Long-RInChIKey=, Short-RInChIKey=, Web-RInChIKey=). The value of the missing fields ends up as being returned as null from RinchiFullInfo, and this was an easy way to (a) pick up the information that is available in those rinchi files and not requiring all of the files to be complete and (b) use the same convenient method genericExampleTest(String,String) for testing.

I fully agree that this is a bit of a hack and I am very happy to solve this differently:

1. Calculate the missing information and amend the rinchi files.
2. Introduce an enum in the class JnaRinchiTest with the five instances RINCHI, RAUXINFO, LONG_RINCHIKEY, SHORT_RINCHIKEY, WEB_RINCHIKEY and add a new method JnaRinchiTest::genericExampleTest(String,String,Enum[]) that explicitly lists the fields that should be compared.
3. Any other idea that you can come up with to make this more robust and solid 😃

[ntk73]

@uli-f , I suppose you have seen that PR #10 was closed and we have created a new clean PR #16. You have added several new test cases and corresponding recourse files. This is great.

Thank you. I am all for putting in as many tests as possible!

The same with me :-)

I have one small remark concerning the small changes of previous test cases: you have added several checks for null pointer before junit assert test. I do not think this check is needed because if something goes wrong the junit test will fail with a null pointer exception for instance - and this is the main goal of the junit test to catch what is wrong. Actually, with a such null pointer check, it is possible some problems to stay hidden because assert is not performed.

I assume you refer to the code in JnaRinchiTest::genericExampleTest(String,String):

if (rfi.getAuxInfo() != null) {
      assertEquals(rfi.getAuxInfo(), rinchiOut.getAuxInfo(), "RAuxInfo for " + reactionFile);
}

I did this because some of the rinchi files (e.g., two_reactants_no_products.rxn.rinchi_strings.txt) I added did not have all the information/fields (RInChI=, RAuxInfo=, Long-RInChIKey=, Short-RInChIKey=, Web-RInChIKey=). The value of the missing fields ends up as being returned as null from RinchiFullInfo, and this was an easy way to (a) pick up the information that is available in those rinchi files and not requiring all of the files to be complete and (b) use the same convenient method genericExampleTest(String,String) for testing.

I fully agree that this is a bit of a hack and I am very happy to solve this differently:

Now when reconsidering, I am OK with this hack. Actually, the main goal is to test the RInChI functionality rather to check the fullness of test recourse files

1. Calculate the missing information and amend the rinchi files.

2. Introduce an enum in the class `JnaRinchiTest` with the five instances `RINCHI`, `RAUXINFO`, `LONG_RINCHIKEY`, `SHORT_RINCHIKEY`, `WEB_RINCHIKEY` and add a new method `JnaRinchiTest::genericExampleTest(String,String,Enum[])` that explicitly lists the fields that should be compared.

3. Any other idea that you can come up with to make this more robust and solid smiley

Very kind of you for suggesting these. In line with the above comment of mine, these are not so crucial and the testing set can stay as it is for now.

[uli-f]

Closed as PR was merged.