1 2 Previous Next 18 Replies Latest reply on Feb 3, 2008 6:02 AM by Aleksandar Kostadinov

    some enhancements to jboss-test

    Aleksandar Kostadinov Novice

      Hi,

      To adopt jboss-test for CI builds I needed to do some changes in addition to JBAS-2235:

      1. Ability to provide username and password so shutdown can work with auth.
      2. Add an optional attribute to the server:stop task "failonerror" which defaults to false right now.
      3. Add useful error message if reference to server manager cannot be found.

      Default behavior has NOT changed So the modification should not affect current users.

      Any complaints/suggestions welcome. Otherwise going to commit these changes ;)

      See svn diff on http://jira.jboss.com/jira/secure/attachment/12317492/jboss-test-diff

        • 1. Re: some enhancements to jboss-test
          ryan.campbell Expert

          This looks good.

          Do you mind adding a retry that uses the -H option if the first request fails?

          • 2. Re: some enhancements to jboss-test
            Aleksandar Kostadinov Novice

            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?

            • 3. Re: some enhancements to jboss-test
              ryan.campbell Expert

              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 ?

              I think your suggestion about jboss.server.shutdown.fail makes sense.

              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.

              I'm fine with you making these changes.

              • 4. Re: some enhancements to jboss-test
                Aleksandar Kostadinov Novice


                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.

                • 5. Re: some enhancements to jboss-test
                  Aleksandar Kostadinov Novice

                   

                  "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 ?


                  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

                  "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.


                  That makes sense!

                  "ryan.campbell@jboss.com" wrote:

                  I'm fine with you making these changes.


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

                  • 6. Re: some enhancements to jboss-test
                    Martin Vecera Novice

                     

                    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.


                    I made the change for destroying the JBoss process not to break the test suite execution (if some stop-task's parameter is set) - JBAS-2235.

                    • 7. Re: some enhancements to jboss-test
                      Aleksandar Kostadinov Novice

                      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.

                      • 8. Re: some enhancements to jboss-test
                        ryan.campbell Expert

                        Good idea so long as it goes somewhere in the output directory.

                        • 9. Re: some enhancements to jboss-test
                          Aleksandar Kostadinov Novice

                          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?

                          • 10. Re: some enhancements to jboss-test
                            ryan.campbell Expert

                            Looking at build.log seems better to me too. Let's not worry about the filenames as you suggest. But we should be sure that we don't fail the build if build.log is missing for some reason.

                            We need to do a release of the jboss-test project and add that release into /trunk only.

                            • 11. Re: some enhancements to jboss-test
                              Aleksandar Kostadinov Novice

                              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.

                              • 13. Re: some enhancements to jboss-test
                                Aleksandar Kostadinov Novice

                                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."

                                • 14. Re: some enhancements to jboss-test
                                  Aleksandar Kostadinov Novice

                                  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."


                                  1 2 Previous Next