Thanks Jeremy. You can go ahead and check in the changes and then update
the JIRA issue.
On 8/1/2012 5:31 AM, Lv Songping wrote:
> Dear Hong Zhang:
>
>> As the tests have run successfully on linux for you, you don't need to
>> wait to figure out the windows failures before you check in the fix for
>> the issue. But one thing I want to mention is that there is a separate
>> set of the tests called QuickLook, they are different from the
>> deployment dev tests. And the Quicklook tests are needed for every check
>> in for all areas. Sorry I did not make that clear initially. I could not
>> find the latest instructions to run Quicklook with GlassFish v4, but the
>> instructions to run it on v3 is similar, just the tests directory is a
>> little different (as the workspace directory has been re-organized in
>> v4). The tests are in the main source repository with other source code,
>> and in directory main/appserver/tests/quicklook (for v3, it's in
>> v3/tests/quicklook).
>> https://wikis.oracle.com/display/GlassFish/CheckInBestPractice
>> step 4 links to the v3 QL test instructions. For this set of the
>> changes, you should only need to run the tests with the Glassfish
>> distribution/profile.
> Thanks for your advices, I have run the QL tests related to the Glassfish
> profile
> in the directory main/appserver/tests/quicklook and all of
> the test cases are passed. I have uploaded my test results
> to my github(https://github.com/LvSongping/GLASSFISH-18916)
>
> QL test information:
> Test environment: Linux RHEL5-x32
> Glassfish trunk: until 2012/07/30
> QuickLook tests version: until 2012/7/30
> scenarios: Glassfish Profile
> Passed cases/Tests number: 107/107
>
> Dev tests Information:
> Test environment: Linux RHEL5-x32
> Glassfish trunk: until 2012/07/30
> QuickLook tests version: until 2012/7/30
> DEPL_TARGET: PE
> Passed cases/Tests number: 263/263
>
>
> --Best Regards
> --Jeremy Lv
>
> -----Original Message-----
> From: Hong Zhang [mailto:hong.hz.zhang_at_oracle.com]
> Sent: Tuesday, July 31, 2012 11:02 PM
> To: Lv Songping
> Cc: dev_at_glassfish.java.net
> Subject: Re: About GLASSFISH-18916
>
> Hi, Jeremy
>> I have run the deployment dev tests and all of the cases related to
>> the environment of PE are PASSED on the environment of LINUX just as the
>> EXPECTED_RESULTS.txt shows.
>> I have uploaded the QuickLook tests results to my github
>> (https://github.com/LvSongping/GLASSFISH-18916 ).
>>
>> Quicktests Information:
>> Test environment: Linux RHEL5-x32
>> Glassfish trunk: until 2012/07/30
>> QuickLook tests version: until 2012/7/30
>> DEPL_TARGET: PE
>> Passed cases/Tests number: 263/263
>>
>> I have run the tests about the deployment on the environment of PE.
>> Is it necessary to run the tests on the environment of EE?
> For this set of the changes it's not necessary. The EE tests only need
> to be run for any changes that potentially impact cluster deployment.
>> However, I still have some questions about the QuickLook tests
>> when the tests runs on the environment of windows before I
>> modified this issue. There will be four cases failed in the content of
>> versioning(test_home\appserv-tests\devtests\deployment\versioning),
>> these four cases are related to the autodeploy.
>>
>> I investigate the reasons why it's failed during autodeploy.
>> The basic logical is as follows:
>> Step1. I found that the jar file name like
>> "simple-versioned-ejb:ALPHA-1.0.1.jar" can't be created
>> because the symbol like ":" can't be used in naming files on
>> the environment of windows. It will create a file called
>> "simple-versioned-ejb" instead.
>> Step 2. It will copy the "simple-versioned-ejb" to the content of
>> "glassfish_home\domains\domain1\autodeploy".
>> Step 3. The operation of autodeploy " simple-versioned-ejb " is failed
>> And then the tests related to the autodeploy will be failed.
> I don't usually use windows so I am not too sure. But I think these
> tests were executed successfully on windows when they were first added?
>> If it is possible, I think I can modified the resource of QuickLook tests
>> so that the autodeploy can be run successfully on the environment of
>> windows.
> If you have some proposed fixes that will be great. But the tests would
> still need to be testing the versioning deployment, not just testing
> regular deployment.
>
> As the tests have run successfully on linux for you, you don't need to
> wait to figure out the windows failures before you check in the fix for
> the issue. But one thing I want to mention is that there is a separate
> set of the tests called QuickLook, they are different from the
> deployment dev tests. And the Quicklook tests are needed for every check
> in for all areas. Sorry I did not make that clear initially. I could not
> find the latest instructions to run Quicklook with GlassFish v4, but the
> instructions to run it on v3 is similar, just the tests directory is a
> little different (as the workspace directory has been re-organized in
> v4). The tests are in the main source repository with other source code,
> and in directory main/appserver/tests/quicklook (for v3, it's in
> v3/tests/quicklook).
>
> https://wikis.oracle.com/display/GlassFish/CheckInBestPractice
>
> step 4 links to the v3 QL test instructions. For this set of the
> changes, you should only need to run the tests with the Glassfish
> distribution/profile.
>
> And do you usually use windows platform? If yes, one area that you could
> contribute and will be very helpful for us is to investigate some of the
> windows specific issues. If you are interested, I can point you to some
> of the issues you could look or you could search in JIRA under
> deployment category, usually the subject/description will indicate if
> the issue is only found on windows.
>
> Thanks,
>
> - Hong
>
>> -----Original Message-----
>> From: Hong Zhang [mailto:hong.hz.zhang_at_oracle.com]
>> Sent: Wednesday, July 25, 2012 11:06 PM
>> To: dev_at_glassfish.java.net
>> Subject: Re: About GLASSFISH-18916
>>
>> The changes look good. Please run QuickLook tests and deployment dev
>> tests to make sure the changes do not introduce any regressions (for
>> this set of changes, deployment dev tests in PE mode should be
> sufficient).
>> Intructions to deployment dev tests:
>> https://wikis.oracle.com/display/GlassFish/4.0DeploymentDevTests
>>
>> Thanks,
>>
>> - Hong
>>
>> On 7/25/2012 5:50 AM, Lv Songping wrote:
>>> Dear Hong Zhang:
>>>
>>>> Thanks for making the changes. Please see my comments below:
>>>> 1. The modified files are in deployment/admin module and not in
>>>> core/kernel.
>>>> 2. For DeployCommand:
>>>> a. As we are already throwing the exception in the if block, we don't
>>>> need to put the rest of the code in the else block here.
>>>> b. For error message, let's do something else
>>>> lifecyclemodule_withsamename_exists=Lifecycle module with same name {0}
>>>> already exists, please pick a different name for the application.
>>>> 3. For CreateLifecycleModuleCommand:
>>>> a. For the import statement, just import the new class you are using
>>>> (Applications) and not import everything
>>>> b. Please don't change the REST endpoints as this is not the deploy
>>>> command.
>>>> c. For the checks you made in validateTarget, you can just add the check
>>>> in the beginning of this method or even outside of the method (you don't
>>>> really need to go into each block of this method and do similar
>>>> validation as this check is not relevant to target anyways, the name has
>>>> to be unique per domain).
>>>> Application app = apps.getApplication(name);
>>>> if(app != null && !app.isLifecycleModule()){
>>>> // throw illegal exception here
>>>> }
>>>> with error string like this:
>>>> application_withsamename_exists=Application with same name {0} already
>>>> exists, please pick a different name for the lifecycle module.
>>> Thanks for your advices; I have updated my github under your suggestions.
>>> Please check it again.
>>>
>>> --Best Regards
>>> --Jeremy Lv
>>> -----Original Message-----
>>> From: Hong Zhang [mailto:hong.hz.zhang_at_oracle.com]
>>> Sent: Tuesday, July 24, 2012 3:00 AM
>>> To: dev_at_glassfish.java.net
>>> Subject: Re: About GLASSFISH-18916
>>>
>>> Hi, Jeremy
>>> Thanks for making the changes. Please see my comments below:
>>> 1. The modified files are in deployment/admin module and not in
>>> core/kernel.
>>> 2. For DeployCommand:
>>> a. As we are already throwing the exception in the if block, we don't
>>> need to put the rest of the code in the else block here.
>>> b. For error message, let's do something else
>>> lifecyclemodule_withsamename_exists=Lifecycle module with same name {0}
>>> already exists, please pick a different name for the application.
>>> 3. For CreateLifecycleModuleCommand:
>>> a. For the import statement, just import the new class you are using
>>> (Applications) and not import everything
>>> b. Please don't change the REST endpoints as this is not the deploy
>>> command.
>>> c. For the checks you made in validateTarget, you can just add the check
>>> in the beginning of this method or even outside of the method (you don't
>>> really need to go into each block of this method and do similar
>>> validation as this check is not relevant to target anyways, the name has
>>> to be unique per domain).
>>> Application app = apps.getApplication(name);
>>> if(app != null && !app.isLifecycleModule()){
>>> // throw illegal exception here
>>> }
>>> with error string like this:
>>> application_withsamename_exists=Application with same name {0} already
>>> exists, please pick a different name for the lifecycle module.
>>>
>>> - Hong
>>>
>>> On 7/23/2012 4:27 AM, Lv Songping wrote:
>>>> Dear Hong Zhang:
>>>>
>>>> I have revised the issue(GLASSFISH-18916) about the " Create a
>>>> LifecycleModule, then deploy a application Module that has the same name
>>>> with the LifecycleModule, NullPointerException happened " under your
>>> advices
>>>> and reflected the modified files into
>>>> https://github.com/LvSongping/GLASSFISH-18916
>>>>
>>>> please review it again.
>>>>
>>>>
>>>> --best regards
>>>> --Jeremy Lv
>>>>
>>>> ÒÔÉÏ
>>>> ÂÀËÎƽ
>>>> ----------------------------------------------------
>>>> Lv Songping
>>>> Development Dept.II
>>>> No. 6 Wenzhu Road, Nanjing, 210012, China
>>>> TEL: +86+25-86630566-8196
>>>> COINS: 7998-8196
>>>> FAX: +86+25-83317685
>>>> Mail: lvsongping_at_cn.fujitsu.com
>>>> ----------------------------------------------------
>>>> This communication is for use by the intended recipient(s) only and may
>>>> contain information that is privileged, confidential and exempt from
>>>> disclosure under applicable law. If you are not an intended recipient of
>>>> this communication, you are hereby notified that any dissemination,
>>>> distribution or copying hereof is strictly prohibited. If you have
>>> received
>>>> this communication in error, please notify me by reply e-mail,
>>> permanently
>>>> delete this communication from your system, and destroy any hard copies
>>> you
>>>> may have printed.
>>>>
>>>> -----Original Message-----
>>>> From: Hong Zhang [mailto:hong.hz.zhang_at_oracle.com]
>>>> Sent: Thursday, July 19, 2012 10:56 PM
>>>> To: dev_at_glassfish.java.net
>>>> Subject: Re: About GLASSFISH-18916
>>>>
>>>> Hi Jeremy
>>>> Thanks for looking into this issue. I think we need to do some
>>>> thinking first on this issue on what is the desirable behavior here.
>>>> Normally when you redeploy an application, it undeploys the old
>>>> version of the application, and deploys the new version of the
>>>> application. But here the old version of the application and the new
>>>> version of the application are very different, one is a lifecycle module
>>>> and the other is a regular Java EE application, and the code paths for
>>>> deploy and undeploy lifecycle module and Java EE application are quite
>>>> different also. Two options here, one is to delete the lifecycle module
>>>> as part of the undeployment, and then deploy the new application. The
>>>> other option is to disallow this type of redeployment, and print a
>>>> meaningful error message to user.
>>>> I prefer the second option as it is the simpler approach (without
>>>> needing to write a lot of code), and should be sufficient to handle this
>>>> not too common use case. The checks would need to be made from both
>>>> sides for handling the two cases (create a lifecycle module first, then
>>>> deploy an application with same name, or deploy an application first,
>>>> and then create a lifecycle module with same name), so you would need to
>>>> change both CreateLifecycleModuleCommand and DeployCommand. For
>>>> DeployCommand, the check could be made in the handleRedeploy method
>>>> after we get the current app. And in CreateLifecyleModuleCommand, the
>>>> check could be made inside the validateTarget method.
>>>>
>>>> Thanks,
>>>>
>>>> - Hong
>>>>
>>>> On 7/18/2012 10:28 PM, Lv Songping wrote:
>>>>> Dear Hong Zhang
>>>>> Cc:Glassfish deployment team
>>>>>
>>>>> I have revised the issue(GLASSFISH-18916) about the " Create a
>>>>> LifecycleModule, then deploy a application Module that has the same
> name
>>>>> with the LifecycleModule, NullPointerException happened " and reflected
>>>> the
>>>>> modified files into https://github.com/LvSongping/GLASSFISH-18916 and
>>>>> please review it and give me some advices.
>>>>>
>>>>> The NPE is occurred because of the LifecycleModule don't have the
>>>> attribute
>>>>> named "Location", when execute the " uri = new
>>>>> URI(application.getLocation())" in UndeployCommand.java, it will throw
>>> out
>>>>> the NPE.
>>>>>
>>>>> The issue url is as follows:
>>>>> http://java.net/jira/browse/GLASSFISH-18916
>>>>>
>>>>> --Best Regard!
>>>>> --Jeremy Lv
>>>>>
>>>>>
>