Open CSchoel opened 3 years ago
- If the omc-java-api throws any exception, report the error message back to the user.
omc-java-apis sendExpression does not throw any Exceptions.
I created a few testcases for this functionality.
Of course sendExpression5 fails because simulation takes different amount off time everytime...
sendExpression9 fails because wrong username...
im not sure what the problem with sendExpression8 is.. (also fails on my machine)
im quite sure that at least the sendExpression4 Error is because of ModelicaVersion 3.2.3 which gets loaded in a test prior and was not used to produce expected output.
PS.: The sendExpression Links somehow don't jump to the correct line when clicked... RightClick -> copy link works fine
The Problem with sendExpression4 is not because of different ModelicaStandardLibrary but because of loading any ModelicaStandardLibrary... I will adjust the expected output. I replaced the expected Output for sendExpression5 with a bad Regex... its ugly but i think it works
Two Questions remain:
Why is does sendExpression8 not work?
expected: <[<interactive>:1:1-1:18:writable] Error: Class unknownAPIMethod not found in scope <global scope> (looking for a function or record).
>
but was: <[<interactive>:1:1-1:0:writable] Error: Class unknownAPIMethod not found in scope <global scope> (looking for a function or record).>
Where does the Notification during sendExpression9 come from?
Notification: Downloaded package index from URL https://libraries.openmodelica.org/index/v1/index.json.
omc-java-apis sendExpression does not throw any Exceptions.
It does not declare any Exceptions in the header, but that does not mean that it wont throw runtime exceptions, those are two different things. If you look at the implementation of sendExpression()
, you can clearly see that the author did anticipate runtime exceptions (otherwise the try
is meaningless) and even throws an IllegalStateException
explicitly.
It was a good idea to implement the test cases. This allows us to discuss test design:
sendExpression4
(which is called executeCommand4
in the code - I assume to achieve maximum confusion ;P) actually tries to accomplish. If you instead name this test method something like retrievedSimulationOptionsForValidModelMatchesExperimentAnnotation
there is no confusion anymore. The name is quite lengthy, but I would argue that the increased clarity justifies the clunky name. If you do not agree, you can shorten the name to something like getSimulationOptionsValid
, but then you should put the full explanation in a Javadoc comment.
Edit: For full disclosure: This is not my own idea. I got it from StackOverflow and followed it for my own projects, because I found the arguments convincing. The main reason to put the explanation in the name is that you do not have to look at the code to know what failed, you can already see it from the test summary.sendExpression5
should probably only test that the result contains the string "The simulation finished successfully"
, as this test only cares about successful simulation and not, for example, simulation performance.sendExpression6
and sendExpression7
reveal another general pitfall during testing: They rely on the order in which the tests are executed. In general, you can never be sure that the order will stay the same and users might even want to just run a single test case out of the suite. Therefore, you have to ensure that all your test cases are completely independent of each other. You can allow some exceptions to this rule as, for example, setting up a new OMC instance for each test would require a large overhead, but there is no harm in, for example, (re-)loading the MSL before the simulate test, or reducing the cd()
test to just checking that a valid file system path is returned - independent of what that path might be. As a general rule: Only test the behavior that is apparent from the specifications of the functions that you call. If you only know that cd()
has been called, you should not make any assumptions about which path will be returned.Two Questions remain:
- Why is does sendExpression8 not work?
expected: <[<interactive>:1:1-1:18:writable] Error: Class unknownAPIMethod not found in scope <global scope> (looking for a function or record). > but was: <[<interactive>:1:1-1:0:writable] Error: Class unknownAPIMethod not found in scope <global scope> (looking for a function or record).>
As you can see, the difference lies in the column number where the error is reported. You expect to see the error from line 1 column 1 to line one column 18, which is the end of the string unknownAPIMethod()
. Instead, you get a strange invalid range from line 1, column 1 to line one, column 0. You could investigate where this comes from, but I would argue that it is inconsequential for the test. This is not a real file we are talking about, but a code snippet that is sent directly to the OMC. What good does a correct line and column do for debugging in this case? I would therefore suggest that you simply make the test more flexible, only checking for the relevant part such as Class unknownAPIMethod not found
.
The downside to this is that with assertTrue(x.contains("..."))
you do not see the actual string content in case the test fails. This can be remedied in two ways:
assertContains()
method that throws an AssertionError
with a more meaningful error message.assertThat
from Hamcrest as shown in this StackOverflow answer.
- Where does the Notification during sendExpression9 come from?
Notification: Downloaded package index from URL https://libraries.openmodelica.org/index/v1/index.json.
The OMC tries to install unknown libraries from a central package index if possible. The "correct" way to do this seems to be to use installLibrary()
, but I think loadModel()
also has a fallback mechanism where it tries to install a library, when it cannot find it on the Modelica path. The error that the package index could not be parsed is a known issue in the OpenModelica project that we will have to work around for now.
sendExpression
that accepts a string in the same format as would be required by the OMShellgetErrorString()
does not return an empty string after the command has been sent, report the results back to the user.loadModel(Modelica, {"3.2.3"})
setCommandLineOptions("-d=newInst,nfAPI")
setCommandLineOptions("--unitChecking")
getSimulationOptions(Modelica.Electrical.Analog.Examples.Rectifier)
simulate(Modelica.Electrical.Analog.Examples.Rectifier)
cd("/home")
cd()