I guess your intention is to not leave the server running till ant exit but allow jboss test suite to complete.
If that's correct, Martin's changes are sufficient since he made server process to be destroyed during stop task. So -H retry would not be needed.

What I don't like with current implementation is that we will not notice shutdown issues in test suite runs. Unfortunately I can't think of a generic way to make the job execute to completion but fail if any servers failed to shutdown. What is easiest to implement is:
1. if server fails to shutdown and failonerror is false (the default), then set a project property "jboss.server.shutdown.fail"
2. have that line after all tests finish ''

Do you want me to do make these changes: #1 in the ant task fine and #2 in the Jboss AS 5 testsuite/build.xml file?

Well, the process.destroy logic doesn't seem to work from what I've seen. So it's basically a useless line.
This is why Dimitris suggested the use of the -H option to try to clean things up. Can you replace the process.destroy with the shutdown -H ?


From my testing of jboss-test/trunk, destroy does work. But it is not yet released and used for AS 5. I think Martin Vecera fixed the logic. Martin, right? AFAICT before Martin's changes it worked on ant exit only effectively breaking the test suite execution.

You could test trunk with /qa/tools/opt/jboss-test-1.0.5-SNAPSHOT.jar


What about setting the value of this property to the name of the server config which fails? That way we can have a line which says:

"The "tomcat-sso-cluster-01 server failed to stop"

Even if we have multiple servers failing to stop, it is enough to know that one failed.


That makes sense!


I'm fine with you making these changes.


Cool, I'll try to do these this week.

"ryan.campbell@jboss.com" wrote:

Well, the process.destroy logic doesn't seem to work from what I've seen. So it's basically a useless line.
This is why Dimitris suggested the use of the -H option to try to clean things up. Can you replace the process.destroy with the shutdown -H ?

"ryan.campbell@jboss.com" wrote:

What about setting the value of this property to the name of the server config which fails? That way we can have a line which says:

"The "tomcat-sso-cluster-01 server failed to stop"

Even if we have multiple servers failing to stop, it is enough to know that one failed.

"ryan.campbell@jboss.com" wrote:

I'm fine with you making these changes.

Using a project property seems not possible since targets are executed using antcall task which creates a subproject. So the property can't be created in the parent project.

The other option is to use a file "jboss.server.shutdown.failed" and fill it with server names failed to shutdown. Then decide whether to fail the build based on it. The file name could be configurable as well.

Actually I see there is already a build.log recorded by the test suite so I can just put a condition to check if any server failed to shutdown. This will not require a change in jboss-test.
I think that is a better solution than bloating the stop task with file modifications.

Extracting server names that failed to shutdown will be harder but if you think it's worth doing, I'll try.

Should I go that way? And please advise which branches should I put that in - Branch_5_0 and trunk?

I can ensure that we don't fail build in such a case, but why should I. I think we should ensure the build.log exists so somebody doesn't disable its creation for no reason. As well that will happen at the end of the build so test results will be recorded anyway.

The check will be inserted after the ant block that generates build.log so I suggest we fail if it doesn't exist. This will not apply to any other existing targets where the check will not be performed at all.

Sorry for delay. Not much time. Please see patch below. I change build.log location as now it doesn't work when target "test" is called because the same file is used with another recorder within the build simultaneously (see init target). So either I've had to change how "maybejars" target gets called in which case the recorder log level would have been changed from what it was by default to error, or introduce a log file for the test target only.

I preferred the second approach as it logs at info level. So allows for more assurance that we're processing the right log. As well it will allow later to implement a check to compare tests executed with test results produced (remember tests overwriting).

Should I disable the build.log generation? I don't believe it ever worked when test target is called (it does for any other targets though)? Or should I move it to output dir as well? Or should I leave it as is?

Index: testsuite/build.xml
===================================================================
--- testsuite/build.xml (revision 68976)
+++ testsuite/build.xml (working copy)
@@ -873,7 +873,7 @@
-->

-
+



@@ -897,7 +897,20 @@



-
+
+
+
+
+
+
+
+
+
+
+
+
+ <fail message="Some servers failed to shutdown cleanly."
+ if="servers.shutdown.failed"/>


<target name="tests-stress" description="Execute all stress tests."

Sorry for delay. Not much time. Please see patch below. I change build.log location as now it doesn't work when target "test" is called because the same file is used with another recorder within the build simultaneously (see init target). So either I've had to change how "maybejars" target gets called in which case the recorder log level would have been changed from what it was by default to error, or introduce a log file for the test target only.

I preferred the second approach as it logs at info level. So allows for more assurance that we're processing the right log. As well it will allow later to implement a check to compare tests executed with test results produced (remember tests overwriting).

Should I disable the build.log generation? I don't believe it ever worked when test target is called (it does for any other targets though)? Or should I move it to output dir as well? Or should I leave it as is?

Index: testsuite/build.xml
===================================================================
--- testsuite/build.xml (revision 68976)
+++ testsuite/build.xml (working copy)
@@ -873,7 +873,7 @@
 -->
 <target name="tests" description="Execute all non-benchmark tests."
 depends="maybejars">
- <record name="${basedir}/build.log" append="no" action="start" loglevel="${buildlog.level}"/>
+ <record name="output/tests.log" append="no" action="start" loglevel="${buildlog.level}"/>
 <property name="nojars" value="true"/>
 <antcall target="jboss-minimal-tests" />
 <antcall target="jboss-all-config-tests"/>
@@ -897,7 +897,20 @@
 <antcall target="tests-aop-scoped"/>
 <antcall target="tests-classloader-leak"/>
 <antcall target="tests-report"/>
- <record name="${basedir}/build.log" action="stop"/>
+ <record name="output/tests.log" action="stop"/>
+ <condition property="servers.shutdown.failed">
+ <and>
+ <not><isset property="servers.shutdown.nocheck"/></not>
+ <isfileselected file="output/tests.log">
+ <or>
+ <contains text="Unable to shutdown server properly"/>
+ <not><contains text="[server:stop]"/></not>
+ </or>
+ </isfileselected>
+ </and>
+ </condition>
+ <fail message="Some servers failed to shutdown cleanly."
+ if="servers.shutdown.failed"/>
 </target>

 <target name="tests-stress" description="Execute all stress tests."