sot / starcheck

BSD 3-Clause "New" or "Revised" License
3 stars 0 forks source link

set exit code 0 at the end #403

Closed javierggt closed 1 year ago

javierggt commented 1 year ago

Description

This PR sets the exit code to 0 at the end of execution.

I can confirm that the current master version return code is not zero. This causes test_regress.sh to fail. Someone who is more familiar with perl should check this.

Interface impacts

None

Testing

Unit tests

Functional tests

I checked that the program runs and returns the given value using the same command as in test_regress.sh:

starcheck -dir $SKA/data/ska_testr/test_loads/2019/MAY2019/oflsa

I did not check an error elsewhere to make sure the error is properly set. I would need to know what error to check for.

jeanconn commented 1 year ago

I think what we want is non-zero exit code if the perl exits via any of the "die" statements and a zero exit code otherwise. Not sure if setting the exit code at the end does that and/or if the END block should preserve and re-spit-out the exit code if it gets there via die. Will look at this some more.

jeanconn commented 1 year ago

Also this code does work to output zero but it didn't start with master with the merges that are the problem.

javierggt commented 1 year ago

Also this code does work to output zero but it didn't start with master with the merges that are the problem.

That's right. It started at another commit. I rebased it.

javierggt commented 1 year ago

Now looking better at the code it is obvious which statement is the one that sets the RC code (to 9):

    kill 9, $pid;                    # must it be 9 (SIGKILL)?
        my $gone_pid = waitpid $pid, 0;  # then check that it's gone
jeanconn commented 1 year ago

I saw that nine but I wasn't 100% sure it was setting the exit code. Either way, my suggestion is in #404 . It seems to work for the success case and for the simplest "die" case of "-dir" not being a products directory.

jeanconn commented 1 year ago

I was reading up more on appropriate handling of errors around END but maybe we don't care about best practices.

jeanconn commented 1 year ago

Superseded by #404