[Pki-devel] [PATCH] 79 Fixes for review comments on patches 74, 75, 76, 77, 78 #CI test framework

[Endi Sukma Dewata]

On 11/9/2013 2:31 AM, Abhishek Koneru wrote:
> Please find the patch with fixed for review comments given by Endi for
> the patches 74,75,76,77,78 as part of the basic CI test framework setup.
>
> The comments addressed are :
>
> --Rename pki/READ_ME_TESTS to pki/tests/dogtag/README
> -- Create sample configuration files for all the config files specified
> in README.
>
> -- Specify all the steps required to setup an environment for running
> the java tests in eclipse.
>
> -- rename the task name for tests in the beaker-job.xml.template
> to /CoreOS/dogtag/PKI_TEST_USER_ID.
>
> Patch 79 has to be applied after applying patches 74,75,76,77,78.
> All the patches, on ACK, will be pushed to master which closes tickets
> 657,722,723,724,725,785 in the 10.1 milestone.
>
> Thanks,
> Abhishek

Thanks for the fixes. Just to clarify, I reviewed mostly patches #75, 
#76 (excluding JUnit tests), #77, and #79.

As discussed before, the Beaker test cases (patch #74), JUnit test cases 
(patch #76), and Jenkins setup (patch #78) will be reviewed separately 
afterwards. The Beaker test execution indicates there are quite a few 
failures.

Based on the initial review, there are a number of issues that need 
further discussions. Please send the list whenever it's ready.

For now all of the patches are ACK'd. There are just a few more minor 
issues below. Once they are fixed they can be pushed.

1. In the README section 1.1.1 let's use pki-tomcat as the default DS 
instance name to match Dogtag's default instance name:

   slapd.ServerIdentifier=pki-tomcat

2. The following line after setup-ds.pl is not indented correctly:

   Then a CA instance ...

3. The beakerjob.rhcs.xml.template still has 'rhcs' in the file name. 
Can it be renamed to beakerjob.dogtag.xml.template or just 
beakerjob.xml.template?

4. In the client.conf right now all parameters are commented out. I 
think we should provide a default authentication, probably the 
username/password auth. So the HUB_URL, AUTH_METHOD=password, USERNAME, 
and PASSWORD should be uncommented by default. The README should be 
updated too.

5. In the README section 1.2.2.1 the "User-ID_for_personalization" can 
we replaced with "unique_label", or something like that.

6. The "Job_xml_config_file" could be replaced with "beaker_job_config" 
and mention that the file is available at conf/beaker-job.cfg.

7. There's also an old path in that section:

   pki/tests/beaker/rhcs/Makefile.

8. The example in compose_function still has reference to internal 
machine and it uses a wrong parameter name. It's better to remove this 
example since we already provide repository.cfg

9. The Create_repo_after_build() creates repos for SOURCES and SPECS 
folders. These are probably unnecessary since they don't contain RPM files.


Please add the followings to the list of issues that need further 
discussions:

10. Copying into /tmp and giving everybody a read access might not be a 
safe way to pass ca_admin_cert.p12, but for test this is probably ok. A 
better way is probably to copy into the test user's home directory and 
change the file ownership to the test user.

   cp /root/.dogtag/pki-tomcat/ca_admin_cert.p12 
$TEST_HOME/.dogtag/pki-tomcat/ca_admin_cert.p12
   chown $TEST_USER.$TEST_GROUP 
$TEST_HOME/.dogtag/pki-tomcat/ca_admin_cert.p12

11. The test user can create the NSS DB in the user's home directory too 
(e.g. ~/.dogtag/nssdb). This will require customizing test.cfg though.

   mkdir ~/.dogtag/nssdb
   certutil -N -d ~/.dogtag/nssdb

-- 
Endi S. Dewata