Summary restart load fix

[joakim-hove]

Task
Bug when loading a restarted summary case when the restarted source was given as an absolute path.

The first commit is the fix.

The two middle commits are test refactoring, the last commit is a test of the fix.

NB: This fix unearthed another bug/problem which comes in the Statoil testdata. That problem is fixed here: #319.

[pgdr]

I think you need to explain what you are thinking here.

[joakim-hove]

This was to reproduce / test a reasonably contrived bug. Have added documentation describing the situation which used to fail.

[pgdr]

What is scope() for?

[joakim-hove]

What is scope() for?

Weird construction - removed; better implemented with a function.

[markusdregi]

I'm working on this in-between other things, so the pace is not exactly hyper speed. But, I can confirm that I reproduce the exact same failure locally on my machine. I'll start digging now..

[markusdregi]

It is test_restart_abs_path test that fails heavily.

Here (https://github.com/joakim-hove/libecl/blob/67e9bebe925631086743c06491bf68c94124bb01/lib/ecl/ecl_smspec.c#L508) restart_case is the file named HISTORY, but as an absolute path. Which makes we think that that line is not comparing what it should be comparing. It returns a NULL-pointer, letting all kind of monsters out of the box when createEclSum is adding the variables.

I'm expecting that the reason it kicks in now is because you are interpreting the paths now as absolute paths. And that it kicks in only on macOS because the prefix of any tmp-path is longer on macOS then on linux.

I hope that helps ;)

[joakim-hove]

It is test_restart_abs_path test that fails heavily.

Thank you - hopefully I can take it from here.

[markusdregi]

No problem. If you just enforce the path in the test to be more than 64 characters long it should burn on your machine as well.. If you do not figure it out, we can discuss it further in Thursday :)

[joakim-hove]

Thank you - this is a semi-old bug which was unearthed now.

It returns a NULL-pointer, letting all kind of monsters out of the box when createEclSum is adding the variables.

It should be:

if (restart_case) {
    if (strlen(restart_case) <= (SUMMARY_RESTART_SIZE * ECL_STRING8_LENGTH))
      ecl_smspec->restart_case = util_alloc_string_copy( restart_case );
}

i.e. nothing should be done if the test fails - that is actually semi-ok.

[joakim-hove]

OK @pgdr - I have tried to address your comments and now this green on Travis - further comments?

[pgdr]

No further comments, good work!

[joakim-hove]

#319 has been rebased on top of this - they need to be merged together. Closing this