Re: [Freeipa-devel] [PATCH] 0269 Add man pages for testing tools

2013-08-29 Thread Rob Crittenden

Petr Viktorin wrote:

On 08/28/2013 12:02 AM, Rob Crittenden wrote:

Petr Viktorin wrote:

Hello,

This patch adds man pages for testing tools.
As far as I can see, we use autotools for installing man pages. I added
the autotools machinery to ipatests/man only. I'd appreciate if an
autotools expert could check if this approach is OK.
Or would it be better to not use autotools at all here?

https://fedorahosted.org/freeipa/ticket/3855 (part 5)


Thanks for the review!


You don't have any man pages in section 8 so that can be removed from
Makefile.am.


Removed


You need to add a line break for the various ways to run the commands.

ipa-test-config [options]
ipa-test-config [options] --global
ipa-test-config [options] hostname

renders as

ipa-test-config   [options]ipa-test-config[options]--global
ipa-test-config [options] hostname


Added


ipa-test-config lacks a header.


Which header do you mean? I see the same header as on the other pages.


ipa-test-config doesn't say where the configuration is stored.


It is not stored anywhere; it's read from environment variables and
printed to stdout. I've clarified the description a bit.


ipa-test-task, in the install-topo description drop the word Please.


Removed


Almost none of the 72 options to ipa-run-test are documented in the man
page.


These are taken from the nosetests command and documented in
nosetests(1). Also, the list can change depending on what plugins are
installed.
I think pointing the reader to nosetests(1) is enough.


rob

It's a shame the test commands don't run in the tree.


Well, they will work in-tree if you set PYTHONPATH to the tree.

For example these work without the packages installed:
PYTHONPATH=. ./ipatests/ipa-run-tests test_ipalib/test_config.py
PYTHONPATH=. ./ipatests/ipa-test-task uninstall-all

You can also point the system-installed ipa-run-tests to in-tree tests.
You just need to use an absolute path because it changes the current
directory:
 ipa-run-tests `pwd`/ipatests/test_ipalib/test_config.py



ACK. Please correct the creation date in the man pages before pushing.

rob

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 0269 Add man pages for testing tools

2013-08-29 Thread Petr Viktorin

On 08/29/2013 03:03 PM, Rob Crittenden wrote:

Petr Viktorin wrote:

On 08/28/2013 12:02 AM, Rob Crittenden wrote:

Petr Viktorin wrote:

Hello,

This patch adds man pages for testing tools.
As far as I can see, we use autotools for installing man pages. I added
the autotools machinery to ipatests/man only. I'd appreciate if an
autotools expert could check if this approach is OK.
Or would it be better to not use autotools at all here?

https://fedorahosted.org/freeipa/ticket/3855 (part 5)


Thanks for the review!


You don't have any man pages in section 8 so that can be removed from
Makefile.am.


Removed


You need to add a line break for the various ways to run the commands.

ipa-test-config [options]
ipa-test-config [options] --global
ipa-test-config [options] hostname

renders as

ipa-test-config   [options]ipa-test-config[options]--global
ipa-test-config [options] hostname


Added


ipa-test-config lacks a header.


Which header do you mean? I see the same header as on the other pages.


ipa-test-config doesn't say where the configuration is stored.


It is not stored anywhere; it's read from environment variables and
printed to stdout. I've clarified the description a bit.


ipa-test-task, in the install-topo description drop the word Please.


Removed


Almost none of the 72 options to ipa-run-test are documented in the man
page.


These are taken from the nosetests command and documented in
nosetests(1). Also, the list can change depending on what plugins are
installed.
I think pointing the reader to nosetests(1) is enough.


rob

It's a shame the test commands don't run in the tree.


Well, they will work in-tree if you set PYTHONPATH to the tree.

For example these work without the packages installed:
PYTHONPATH=. ./ipatests/ipa-run-tests test_ipalib/test_config.py
PYTHONPATH=. ./ipatests/ipa-test-task uninstall-all

You can also point the system-installed ipa-run-tests to in-tree tests.
You just need to use an absolute path because it changes the current
directory:
 ipa-run-tests `pwd`/ipatests/test_ipalib/test_config.py



ACK. Please correct the creation date in the man pages before pushing.

rob


Thank you! Corrected  pushed.
master: f742520760d1b146cd3c5e79a6c86a024570ff6a
ipa-3-3: 5945988d7373d65d5a38b11ade84fabe9ac7bb68


--
PetrĀ³

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 0269 Add man pages for testing tools

2013-08-28 Thread Petr Viktorin

On 08/28/2013 12:02 AM, Rob Crittenden wrote:

Petr Viktorin wrote:

Hello,

This patch adds man pages for testing tools.
As far as I can see, we use autotools for installing man pages. I added
the autotools machinery to ipatests/man only. I'd appreciate if an
autotools expert could check if this approach is OK.
Or would it be better to not use autotools at all here?

https://fedorahosted.org/freeipa/ticket/3855 (part 5)


Thanks for the review!


You don't have any man pages in section 8 so that can be removed from
Makefile.am.


Removed


You need to add a line break for the various ways to run the commands.

ipa-test-config [options]
ipa-test-config [options] --global
ipa-test-config [options] hostname

renders as

ipa-test-config   [options]ipa-test-config[options]--global
ipa-test-config [options] hostname


Added


ipa-test-config lacks a header.


Which header do you mean? I see the same header as on the other pages.


ipa-test-config doesn't say where the configuration is stored.


It is not stored anywhere; it's read from environment variables and 
printed to stdout. I've clarified the description a bit.



ipa-test-task, in the install-topo description drop the word Please.


Removed


Almost none of the 72 options to ipa-run-test are documented in the man
page.


These are taken from the nosetests command and documented in 
nosetests(1). Also, the list can change depending on what plugins are 
installed.

I think pointing the reader to nosetests(1) is enough.


rob

It's a shame the test commands don't run in the tree.


Well, they will work in-tree if you set PYTHONPATH to the tree.

For example these work without the packages installed:
   PYTHONPATH=. ./ipatests/ipa-run-tests test_ipalib/test_config.py
   PYTHONPATH=. ./ipatests/ipa-test-task uninstall-all

You can also point the system-installed ipa-run-tests to in-tree tests. 
You just need to use an absolute path because it changes the current 
directory:

ipa-run-tests `pwd`/ipatests/test_ipalib/test_config.py

--
PetrĀ³

From 272d740a7c8514eee5e5cc50036b1b3de77e4775 Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Tue, 13 Aug 2013 18:32:36 +0200
Subject: [PATCH] Add man pages for testing tools

Add man pages for ipa-run-tests, ipa-test-task, and ipa-test-config.

https://fedorahosted.org/freeipa/ticket/3855 (part 5)
---
 .gitignore |   5 ++
 Makefile   |   7 +-
 freeipa.spec.in|   6 ++
 ipatests/man/Makefile.am   |  18 +
 ipatests/man/configure.ac  |  24 +++
 ipatests/man/ipa-run-tests.1   |  63 +
 ipatests/man/ipa-test-config.1 | 157 +
 ipatests/man/ipa-test-task.1   | 126 +
 8 files changed, 405 insertions(+), 1 deletion(-)
 create mode 100644 ipatests/man/Makefile.am
 create mode 100644 ipatests/man/configure.ac
 create mode 100644 ipatests/man/ipa-run-tests.1
 create mode 100644 ipatests/man/ipa-test-config.1
 create mode 100644 ipatests/man/ipa-test-task.1

diff --git a/.gitignore b/.gitignore
index 738b00b13b7014ea915da9bcd06ab1ac4eee3764..5252dad1befa9279b8e5cd033e395309f6a9ae8a 100644
--- a/.gitignore
+++ b/.gitignore
@@ -81,6 +81,11 @@ ipa-client/py-compile
 ipa-client/stamp-h1
 ipa-client/version.m4
 ipatests/test_xmlrpc/service.crt
+ipatests/man/aclocal.m4
+ipatests/man/autom4te.cache/
+ipatests/man/config.status
+ipatests/man/install-sh
+ipatests/man/missing
 freeipa.spec
 ipapython/setup.py
 ipapython/version.py
diff --git a/Makefile b/Makefile
index 674143b8df14e93833d87fcea5740efbb28a9374..a21cf7e33275fd1a783e89baf237c8dcd8db6508 100644
--- a/Makefile
+++ b/Makefile
@@ -71,6 +71,9 @@ client-autogen: version-update
 	cd ipa-client; if [ ! -e Makefile ]; then ../autogen.sh --prefix=/usr --sysconfdir=/etc --localstatedir=/var --libdir=$(LIBDIR); fi
 	cd install; if [ ! -e Makefile ]; then ../autogen.sh --prefix=/usr --sysconfdir=/etc --localstatedir=/var --libdir=$(LIBDIR); fi
 
+tests-man-autogen: version-update
+	cd ipatests/man; if [ ! -e Makefile ]; then ../../autogen.sh --prefix=/usr --sysconfdir=/etc --localstatedir=/var --libdir=$(LIBDIR); fi
+
 install: all server-install tests-install
 	@for subdir in $(SUBDIRS); do \
 		(cd $$subdir  $(MAKE) $@) || exit 1; \
@@ -153,15 +156,17 @@ server-install: server
 		$(PYTHON) setup.py install --root $(DESTDIR); \
 	fi
 
-tests: version-update
+tests: version-update tests-man-autogen
 	cd ipatests; $(PYTHON) setup.py build
+	cd ipatests/man  $(MAKE) all
 
 tests-install: tests
 	if [ $(DESTDIR) =  ]; then \
 		cd ipatests; $(PYTHON) setup.py install; \
 	else \
 		cd ipatests; $(PYTHON) setup.py install --root $(DESTDIR); \
 	fi
+	cd ipatests/man  $(MAKE) install
 
 archive:
 	-mkdir -p dist
diff --git a/freeipa.spec.in b/freeipa.spec.in
index 46a2ae0210c6b75fea971f5d02a10db22815787c..67bd3667db42ebb3639f50a4dd0c3a9fda000781 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ 

Re: [Freeipa-devel] [PATCH] 0269 Add man pages for testing tools

2013-08-27 Thread Rob Crittenden

Petr Viktorin wrote:

Hello,

This patch adds man pages for testing tools.
As far as I can see, we use autotools for installing man pages. I added
the autotools machinery to ipatests/man only. I'd appreciate if an
autotools expert could check if this approach is OK.
Or would it be better to not use autotools at all here?

https://fedorahosted.org/freeipa/ticket/3855 (part 5)


You don't have any man pages in section 8 so that can be removed from 
Makefile.am.


You need to add a line break for the various ways to run the commands.

ipa-test-config [options]
ipa-test-config [options] --global
ipa-test-config [options] hostname

renders as

ipa-test-config   [options]ipa-test-config[options]--global
ipa-test-config [options] hostname

ipa-test-config lacks a header.

ipa-test-config doesn't say where the configuration is stored.

ipa-test-task, in the install-topo description drop the word Please.

Almost none of the 72 options to ipa-run-test are documented in the man 
page.


rob

It's a shame the test commands don't run in the tree.


___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel