Thanks alo Endi. All the comments are addressed. The patches are pushed to master. The tickets that will be closed are 657,722,723,724,725,785 --Abhishek ----- Original Message ----- From: "Endi Sukma Dewata" <edewata@redhat.com> To: "Abhishek Koneru" <akoneru@redhat.com> Cc: "pki-devel" <pki-devel@redhat.com> Sent: Sunday, November 10, 2013 2:27:28 PM Subject: Re: [Pki-devel] [PATCH] 79 Fixes for review comments on patches 74, 75, 76, 77, 78 #CI test framework 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