Set start of 'ok' and improve check of empty 'file' string

[tobolar]

No description provided.

[HansOlsson]

I don't understand this, and those testing functions in general.

To me it indicates a mixup of two different testing variants:

Basically the functions internally assert if there is a failure, and always set ok to true at the end. Some (as this proposed one) set ok to false in the beginning, but I don't see the benefit - either the assert triggers and then it stops and the return value isn't relevant, or the assert doesn't trigger and it returns true.
I would either remove the asserts (probably not) or the return value.

The file part likely didn't work before, and the change doesn't help.
The correct solution would be:

if Modelica.Utilities.Strings.isEmpty(logFile) then
  file:="";
else
  file:=Modelica.Utilities.Files.fullPathName(logFile);
  print("... testAllFunctions(..) is logged in " + file);
  Modelica.Utilities.Files.removeFile(file);
end if;

So, that you send in an empty logfile if you don't want log-files, whereas fullPathName of "" may return the local directory!

[HansOlsson]

Alternatively just don't change 'ok'.

[HansOlsson]

BTW: Why is fullPathName needed at all?

I thought the other commands also handled relative paths as well, and only ModelicaTest.Utilities.Internal changes directory and it should restore it - if that fails the asserts will trigger before anything is logged to the file.

So, just:

if Modelica.Utilities.Strings.isEmpty(logFile) then
  file:="";
else
  file:=logFile;
  print("... testAllFunctions(..) is logged in " + file);
  Modelica.Utilities.Files.removeFile(file);
end if;

or back to:

if logFile<>"" then
  print("... testAllFunctions(..) is logged in " + logFile);
  Modelica.Utilities.Files.removeFile(logFile);
end if;

and replace file by logFile.

[tobolar]

@HansOlsson Well, I didn't digger deeply in this function. The PR is only about to handle empty string of file properly i.e. using given MSL function.
Regarding ok - if the default value of ok is false, then there is no need to change it. You are right. I will revert this.

[HansOlsson]

@HansOlsson Well, I didn't digger deeply in this function. The PR is only about to handle empty string of file properly i.e. using given MSL function.

But there are some problems:

• isEmpty() returns true for both "" and " " (with space), whereas print will only print to the screen if "", so for consistency with print I would keep it as is
• the test isn't for the input String (would be useful if you don't want the file output), but for the result of Modelica.Utilities.Files.fullPathName() - which is very unlikely to be an empty string.
• And as noted I don't know why fullPathName is used at all

So the PR isn't that bad in itself - it just made me realize how messy the function really is.

[tobolar]

@HansOlsson I fully agree to all your three concerns.

So the PR isn't that bad in itself - it just made me realize how messy the function really is.

So I prefer retrieve this PR. Changing the function as you suggest is all fine but shall be done in a new PR.