Hi folks,
looking at the code before falling asleep ....
Regarding the exit code handling we have two driving forces
1) determine a success/failure depending on the operating system
(DefaultExecutor.isFailure())
2) overwrite the OS-specific semantic (DefaultExecutor.isSuccess()) - I
have applications always returning '1' and worked with other
applications that return bit fields to indicate success/warning/errors.
The problem with DefaultExecutor is being too helpful - the default
behavior is to throw an exception so the client usually never see the
failure code.
Looking at the implementation
a) As sebb pointed out the success/failure detection is a little bit odd
under VMS compared to other OS therefore a 'isFailure()' method should
go into the CommandLauncher interface with a special implementation in
VmsCommandLauncher. The DefaultExecutor holds a reference to a
CommandLauncher so it can delegate the check to the CommandLauncher
instance. This would remove this obfuscated 'instanceof' hack (asking
for OS.isFamilyOpenVms() is pretty much the same as 'if (launher
instanceOf VmsCommandLauncher)')
b) Having a isSucess() and isFailure() method is confusing - there
should be only one method name for checking the same one thing whereas I
lean towards isFailure() since this is probably there more common check
in application code.
c) Therefore I wold expose a 'boolean Executor.isFailure(int exitValue)'
to be implemented in DefaultExecutor which would also handle
user-specific overwriting of exit code checking
In short without technobabble (see http://www.pathcom.com/~boby/babble.htm)
+) expose a CommandLauncher.isFailure(int) and implement it in the
implementation classes such as VmsCommandLaunher
+) expose a Executor.isFailure(int) to be implemented in DefaultExecutor
+) fix the regression tests
Any opinions out there
Siegfried Goeschl
Matt Benson wrote:
--- sebb <[EMAIL PROTECTED]> wrote:
On 17/04/2008, Matt Benson <[EMAIL PROTECTED]>
wrote:
--- Siegfried Goeschl <[EMAIL PROTECTED]> wrote:
> Hi sebb,
>
> thanks for your input and I'm sorry for my late
> response - I will look
> at the stuff during the weekend
>
> Siegfried Goeschl
>
> sebb wrote:
> > VMS testing has revealed that the
> DefaultExecutorTest class assumes
> > that 0 = success, and 1 = failure.
> >
> > This is not the case for all OSes - VMS
regards
> odd numbers as
> > successful and even ones as failures.
> >
> > So I think the test needs to use a more
versatile
> means of checking statuses.
> >
> > There is a method in DefaultExecutor
> > public static boolean isFailure(final int
> exitValue)
> > However, it is static, so cannot be added to
the
> Executor interface.
> >
> > Any objections if I make it non-static and
add it
> to the interface?
> > This will allow the test suite to correctly
check
> for success/failure.
> >
> > I suspect the private boolean isSuccess(final
int
> exitValue) method
> > may need updating for VMS. And I don't
understand
> why it returns true
> > if there is no exitValues array - perhaps it
> should return !
> > isFailure(exitValue) ? Or throw an error of
some
> kind?
If the test case exists simply to test
DefaultExecutor, why not explicitly refer to
DefaultExecutor.isFailure()?
Yes, that's also possible.
Or move the static
method into an abstract BaseExecutor superclass
that
hypothetical other Executor implementations could
use
for the same purpose, and refer to the method
there?
Likewise.
However, I think any Executor implementation is
going to need to
provide an isFailure() - or perhaps better an
isSuccess() - method for
callers to be able to check if the execute has
succeeded or not.
Okay, so compromise = add isSuccess() to the interface
and implement in an abstract base class that
encapsulates the VMS exception to the common 0 ->
success rule? :)
-Matt
-Matt
> >
> >
>
---------------------------------------------------------------------
> > To unsubscribe, e-mail:
> [EMAIL PROTECTED]
> > For additional commands, e-mail:
> [EMAIL PROTECTED]
> >
> >
> >
> >
>
>
---------------------------------------------------------------------
> To unsubscribe, e-mail:
> [EMAIL PROTECTED]
> For additional commands, e-mail:
> [EMAIL PROTECTED]
>
>
____________________________________________________________________________________
Be a better friend, newshound, and
know-it-all with Yahoo! Mobile. Try it now.
http://mobile.yahoo.com/;_ylt=Ahu06i62sR8HDtDypao8Wcj9tAcJ
---------------------------------------------------------------------
To unsubscribe, e-mail:
[EMAIL PROTECTED]
For additional commands, e-mail:
[EMAIL PROTECTED]
---------------------------------------------------------------------
To unsubscribe, e-mail:
[EMAIL PROTECTED]
For additional commands, e-mail:
[EMAIL PROTECTED]
____________________________________________________________________________________
Be a better friend, newshound, and
know-it-all with Yahoo! Mobile. Try it now. http://mobile.yahoo.com/;_ylt=Ahu06i62sR8HDtDypao8Wcj9tAcJ
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
