Opened 5 years ago

Last modified 3 years ago

#5724 new defect

Make sure OMEdit doesn't break if inappropriate flags are selected

Reported by: Francesco Casella Owned by: Adeel Asghar
Priority: blocker Milestone: 1.19.0
Component: OMEdit Version:
Keywords: Cc: Oliver Lenord, Per Östlund

Description

OMEdit allows to set any compilation flag by means of the Tools|Options|Simulation|Additional Translation Flags input field.

Ticket #5680 shows that at least in one instance, the use of some flags (in this case -m) can cause errors or incorrect/inappropriate behaviour of OMEdit.

We should analyze all the compilation flags besides -m to figure out which ones break the correct behaviour of OMEdit. Then, we should issue an error when any of them is included in the setup menu.

In case there are some that may still be useful for advanced experimentation or debugging, but potentially dangerous otherwise, we should issue a warning when they are set.

Change History (10)

comment:1 by Adrian Pop, 5 years ago

There is one more I fixed recently: #5689.

comment:2 by Oliver Lenord, 5 years ago

Would it be possible to automate this kind of test?
Assuming that there is limited number of API calls that OMEdit is using to communicate with OMC and for which the return values are known for a given test model, one could create a testing script that performing these API call under all possible compiler settings.

in reply to:  2 comment:3 by Adrian Pop, 5 years ago

Cc: Per Östlund added

@olivleno: there are currently 191 debug flags:
https://github.com/OpenModelica/OpenModelica/blob/master/OMCompiler/Compiler/Util/Flags.mo#L167
and 136 config flags:
https://github.com/OpenModelica/OpenModelica/blob/master/OMCompiler/Compiler/Util/Flags.mo#L1325

The debug flags are true/false but the config flags can have more values.
I don't think is possible to do full coverage of flags as it would take quite a long time.

What we could do is this:

  1. let the user set any flags it wants in OMEdit: Tools|Options|Simulation|Additional Translation Flags
  2. run some quick api tests after the flags are set and check with an expected answer
  3. if the check are fine allow the flags, if the checks are bad, refuse the flags and tell the user why the flags are refused

comment:4 by Oliver Lenord, 5 years ago

Yes I know there are a lot. That's why I thought it will be tedious to analyze all of them one-by-one and rather have a script.

I do not expect that OMEdit is running these tests to check the users settings. I was assuming that this type of interace test becomes part of the test suite. Taking the "-m" bug: Here the text view was obscured by the fact that OMC applied fully qualified names.

This bug could have been caught by making sure that the function that returns the code string is giving expected outputs when called from OMEdit.

comment:5 by Adrian Pop, 5 years ago

We do that in the testsuite, we test the OMEdit<->OMC interface quite a lot but with the default flags. All the tests inside this folder are testing the interactive API and that includes the questions OMEdit asks OMC:
https://github.com/OpenModelica/OpenModelica/tree/master/testsuite/openmodelica/interactive-API
We could try to run all these tests with different flags but that would be quite a lot.

comment:6 by Francesco Casella, 5 years ago

@olivleno, I understand you were bugged by #5680, but I don't think that testing each and every flag is feasible.

BTW, in many cases the flags work in combination, or they take a potentially long list of arguments, see, e.g. --preOptModules, so full coverage is a big deal. Of course there is a finite number of combinations to be tested; however, that will be very large, and it will require a lot of manual work to figure out the meaningful combinations. Hence, I am also convinced that a brute force coverage test is unfeasible.

My proposal was based on the idea that it is possible to determine which flags could be potentially dangerous and which are irrelevant by just looking at them and reasoning for one minute.

For example, --preOptModules or --matchingAlgorithm only have an influence on the backend, so they are obviously irrelevant for the response of the API to OMEdit, and it makes no sense to test for them.

It is possible that after determining that most flags are for sure irrelevant (e.g. all the backend ones), there is a subset left for which we aren't sure. Maybe we should still test them.

comment:7 by Francesco Casella, 4 years ago

Milestone: 1.16.01.17.0

1.16.0 feature freeze is imminent, new developments postponed to 1.17.0

comment:8 by Francesco Casella, 4 years ago

Milestone: 1.17.01.18.0

Rescheduled to 1.18.0

comment:9 by Francesco Casella, 3 years ago

Milestone: 1.18.0

Ticket retargeted after milestone closed

comment:10 by Francesco Casella, 3 years ago

Milestone: 1.19.0

1.18.0 blocker tickets moved to 1.19.0

Note: See TracTickets for help on using tickets.