From bc887f2c69358f6479469ae88c28c4602dcac8d3 Mon Sep 17 00:00:00 2001
From: Tomas Kopecek <tkopecek@redhat.com>
Date: Mar 09 2020 14:03:29 +0000
Subject: [PATCH 1/4] Better help for build/latest-build


People can be confused by buildtag/desttag/target. Shed some light in
these commands' helps.

Fixes: https://pagure.io/koji/issue/1808

---

diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py
index 11353be..07ec7d2 100644
--- a/cli/koji_cli/commands.py
+++ b/cli/koji_cli/commands.py
@@ -11,6 +11,7 @@ import random
 import re
 import stat
 import sys
+import textwrap
 import time
 import traceback
 from collections import OrderedDict, defaultdict
@@ -452,7 +453,20 @@ def handle_remove_pkg(goptions, session, args):
 
 def handle_build(options, session, args):
     "[build] Build a package from source"
-    usage = _("usage: %prog build [options] <target> <srpm path or scm url>")
+
+    usage = _("""\
+        usage: %prog build [options] <target> <srpm path or scm url>
+
+        First option is build target (don't confuse it with destination
+        tag (where build ends) or buildroot (from where dependencies
+        are installed).
+
+        List of all available targets can be acquired by "
+        'koji list-targets'. For further info about how tags, targets "
+        and buildroot interact, check the "
+        https://docs.pagure.org/koji/HOWTO/#package-organization""")
+
+    usage = textwrap.dedent(usage)
     parser = OptionParser(usage=get_usage_str(usage))
     parser.add_option("--skip-tag", action="store_true",
                       help=_("Do not attempt to tag package"))
@@ -2325,8 +2339,15 @@ def handle_revoke_cg_access(goptions, session, args):
 
 
 def anon_handle_latest_build(goptions, session, args):
-    "[info] Print the latest builds for a tag"
-    usage = _("usage: %prog latest-build [options] <tag> <package> [<package> ...]")
+    """[info] Print the latest builds for a tag"""
+    usage = _("""\
+        usage: %prog latest-build [options] <tag> <package> [<package> ...]
+
+        Note, that <tag> needn't be same as build target. If you've wanted to
+        see what was the latest build in given buildroot, check '%prog
+        list-targets --name=<target>' to find name of the buildroot's tag""")
+
+    usage = textwrap.dedent(usage)
     parser = OptionParser(usage=get_usage_str(usage))
     parser.add_option("--arch", help=_("List all of the latest packages for this arch"))
     parser.add_option("--all", action="store_true",
diff --git a/tests/test_cli/test_build.py b/tests/test_cli/test_build.py
index 00459d5..3396c54 100644
--- a/tests/test_cli/test_build.py
+++ b/tests/test_cli/test_build.py
@@ -3,14 +3,11 @@ import mock
 import os
 import six
 import sys
-try:
-    import unittest2 as unittest
-except ImportError:
-    import unittest
 
 from koji_cli.commands import handle_build, _progress_callback
+from . import utils
 
-class TestBuild(unittest.TestCase):
+class TestBuild(utils.CliTestCase):
     # Show long diffs in error output...
     maxDiff = None
 
@@ -22,6 +19,20 @@ class TestBuild(unittest.TestCase):
         self.options.poll_interval = 0
         # Mock out the xmlrpc server
         self.session = mock.MagicMock()
+        self.error_format = """Usage: %s build [options] <target> <srpm path or scm url>
+
+First option is build target (don't confuse it with destination
+tag (where build ends) or buildroot (from where dependencies
+are installed).
+
+List of all available targets can be acquired by "
+'koji list-targets'. For further info about how tags, targets "
+and buildroot interact, check the "
+https://docs.pagure.org/koji/HOWTO/#package-organization
+(Specify the --help global option for a list of other help options)
+
+%s: error: {message}
+""" % (self.progname, self.progname)
 
     @mock.patch('sys.stdout', new_callable=six.StringIO)
     @mock.patch('koji_cli.commands.activate_session')
@@ -147,11 +158,7 @@ Task info: weburl/taskinfo?taskID=1
         actual_stdout = stdout.getvalue()
         actual_stderr = stderr.getvalue()
         expected_stdout = ''
-        expected_stderr = """Usage: %s build [options] <target> <srpm path or scm url>
-(Specify the --help global option for a list of other help options)
-
-%s: error: Exactly two arguments (a build target and a SCM URL or srpm file) are required
-""" % (progname, progname)
+        expected_stderr = self.format_error_message("Exactly two arguments (a build target and a SCM URL or srpm file) are required")
         self.assertMultiLineEqual(actual_stdout, expected_stdout)
         self.assertMultiLineEqual(actual_stderr, expected_stderr)
 
@@ -193,6 +200,15 @@ Task info: weburl/taskinfo?taskID=1
         actual_stdout = stdout.getvalue()
         actual_stderr = stderr.getvalue()
         expected_stdout = """Usage: %s build [options] <target> <srpm path or scm url>
+
+First option is build target (don't confuse it with destination
+tag (where build ends) or buildroot (from where dependencies
+are installed).
+
+List of all available targets can be acquired by "
+'koji list-targets'. For further info about how tags, targets "
+and buildroot interact, check the "
+https://docs.pagure.org/koji/HOWTO/#package-organization
 (Specify the --help global option for a list of other help options)
 
 Options:
@@ -257,11 +273,7 @@ Options:
         actual_stdout = stdout.getvalue()
         actual_stderr = stderr.getvalue()
         expected_stdout = ''
-        expected_stderr = """Usage: %s build [options] <target> <srpm path or scm url>
-(Specify the --help global option for a list of other help options)
-
-%s: error: --arch_override is only allowed for --scratch builds
-""" % (progname, progname)
+        expected_stderr = self.format_error_message("--arch_override is only allowed for --scratch builds")
         self.assertMultiLineEqual(actual_stdout, expected_stdout)
         self.assertMultiLineEqual(actual_stderr, expected_stderr)
 
@@ -352,11 +364,7 @@ Task info: weburl/taskinfo?taskID=1
         with self.assertRaises(SystemExit) as cm:
             handle_build(self.options, self.session, args)
         actual = stderr.getvalue()
-        expected = """Usage: %s build [options] <target> <srpm path or scm url>
-(Specify the --help global option for a list of other help options)
-
-%s: error: Unknown build target: target
-""" % (progname, progname)
+        expected = self.format_error_message( "Unknown build target: target")
         self.assertMultiLineEqual(actual, expected)
         # Finally, assert that things were called as we expected.
         activate_session_mock.assert_called_once_with(self.session, self.options)
@@ -403,11 +411,7 @@ Task info: weburl/taskinfo?taskID=1
         with self.assertRaises(SystemExit) as cm:
             handle_build(self.options, self.session, args)
         actual = stderr.getvalue()
-        expected = """Usage: %s build [options] <target> <srpm path or scm url>
-(Specify the --help global option for a list of other help options)
-
-%s: error: Unknown destination tag: dest_tag_name
-""" % (progname, progname)
+        expected = self.format_error_message("Unknown destination tag: dest_tag_name")
         self.assertMultiLineEqual(actual, expected)
         # Finally, assert that things were called as we expected.
         activate_session_mock.assert_called_once_with(self.session, self.options)
@@ -454,11 +458,7 @@ Task info: weburl/taskinfo?taskID=1
         with self.assertRaises(SystemExit) as cm:
             handle_build(self.options, self.session, args)
         actual = stderr.getvalue()
-        expected = """Usage: %s build [options] <target> <srpm path or scm url>
-(Specify the --help global option for a list of other help options)
-
-%s: error: Destination tag dest_tag_name is locked
-""" % (progname, progname)
+        expected = self.format_error_message("Destination tag dest_tag_name is locked")
         self.assertMultiLineEqual(actual, expected)
         # Finally, assert that things were called as we expected.
         activate_session_mock.assert_called_once_with(self.session, self.options)

From b9c3b46bab06b1488c87b24925b8f914b6c44d8f Mon Sep 17 00:00:00 2001
From: Tomas Kopecek <tkopecek@redhat.com>
Date: Apr 01 2020 11:20:40 +0000
Subject: [PATCH 2/4] fix grammar


---

diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py
index 07ec7d2..9d2a67a 100644
--- a/cli/koji_cli/commands.py
+++ b/cli/koji_cli/commands.py
@@ -457,13 +457,12 @@ def handle_build(options, session, args):
     usage = _("""\
         usage: %prog build [options] <target> <srpm path or scm url>
 
-        First option is build target (don't confuse it with destination
-        tag (where build ends) or buildroot (from where dependencies
-        are installed).
+        The first option is the build target, not to be confused with the destination
+        tag (where the build eventually lands) or build tag (where the buildroot
+        contents are pulled from).
 
-        List of all available targets can be acquired by "
-        'koji list-targets'. For further info about how tags, targets "
-        and buildroot interact, check the "
+        You can list all available build targets using the 'koji list-targets' command.
+        More detail can be found in the documentation.
         https://docs.pagure.org/koji/HOWTO/#package-organization""")
 
     usage = textwrap.dedent(usage)

From ca6ed1254eae9f1b04cd206b0e64b8d827af6fe7 Mon Sep 17 00:00:00 2001
From: Tomas Kopecek <tkopecek@redhat.com>
Date: Apr 01 2020 11:28:01 +0000
Subject: [PATCH 3/4] fix test


---

diff --git a/tests/test_cli/test_build.py b/tests/test_cli/test_build.py
index 3396c54..ac50b28 100644
--- a/tests/test_cli/test_build.py
+++ b/tests/test_cli/test_build.py
@@ -21,13 +21,12 @@ class TestBuild(utils.CliTestCase):
         self.session = mock.MagicMock()
         self.error_format = """Usage: %s build [options] <target> <srpm path or scm url>
 
-First option is build target (don't confuse it with destination
-tag (where build ends) or buildroot (from where dependencies
-are installed).
+The first option is the build target, not to be confused with the destination
+tag (where the build eventually lands) or build tag (where the buildroot
+contents are pulled from).
 
-List of all available targets can be acquired by "
-'koji list-targets'. For further info about how tags, targets "
-and buildroot interact, check the "
+You can list all available build targets using the 'koji list-targets' command.
+More detail can be found in the documentation.
 https://docs.pagure.org/koji/HOWTO/#package-organization
 (Specify the --help global option for a list of other help options)
 
@@ -201,13 +200,12 @@ Task info: weburl/taskinfo?taskID=1
         actual_stderr = stderr.getvalue()
         expected_stdout = """Usage: %s build [options] <target> <srpm path or scm url>
 
-First option is build target (don't confuse it with destination
-tag (where build ends) or buildroot (from where dependencies
-are installed).
+The first option is the build target, not to be confused with the destination
+tag (where the build eventually lands) or build tag (where the buildroot
+contents are pulled from).
 
-List of all available targets can be acquired by "
-'koji list-targets'. For further info about how tags, targets "
-and buildroot interact, check the "
+You can list all available build targets using the 'koji list-targets' command.
+More detail can be found in the documentation.
 https://docs.pagure.org/koji/HOWTO/#package-organization
 (Specify the --help global option for a list of other help options)
 

From 7f27384bb44fbb314c0097a10e5231d4426664a6 Mon Sep 17 00:00:00 2001
From: Mike McLean <mikem@redhat.com>
Date: Apr 02 2020 08:48:12 +0000
Subject: [PATCH 4/4] more grammar changes


---

diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py
index 9d2a67a..44251fc 100644
--- a/cli/koji_cli/commands.py
+++ b/cli/koji_cli/commands.py
@@ -461,7 +461,7 @@ def handle_build(options, session, args):
         tag (where the build eventually lands) or build tag (where the buildroot
         contents are pulled from).
 
-        You can list all available build targets using the 'koji list-targets' command.
+        You can list all available build targets using the '%prog list-targets' command.
         More detail can be found in the documentation.
         https://docs.pagure.org/koji/HOWTO/#package-organization""")
 
@@ -2342,9 +2342,13 @@ def anon_handle_latest_build(goptions, session, args):
     usage = _("""\
         usage: %prog latest-build [options] <tag> <package> [<package> ...]
 
-        Note, that <tag> needn't be same as build target. If you've wanted to
-        see what was the latest build in given buildroot, check '%prog
-        list-targets --name=<target>' to find name of the buildroot's tag""")
+        The first option should be the name of a tag, not the name of a build target.
+        If you want to know the latest build in buildroots for a given build target,
+        then you should use the name of the build tag for that target. You can find
+        this value by running '%prog list-targets --name=<target>'
+
+        More information on tags and build targets can be found in the documentation.
+        https://docs.pagure.org/koji/HOWTO/#package-organization""")
 
     usage = textwrap.dedent(usage)
     parser = OptionParser(usage=get_usage_str(usage))
diff --git a/tests/test_cli/test_build.py b/tests/test_cli/test_build.py
index ac50b28..de1ff4b 100644
--- a/tests/test_cli/test_build.py
+++ b/tests/test_cli/test_build.py
@@ -25,13 +25,13 @@ The first option is the build target, not to be confused with the destination
 tag (where the build eventually lands) or build tag (where the buildroot
 contents are pulled from).
 
-You can list all available build targets using the 'koji list-targets' command.
+You can list all available build targets using the '%s list-targets' command.
 More detail can be found in the documentation.
 https://docs.pagure.org/koji/HOWTO/#package-organization
 (Specify the --help global option for a list of other help options)
 
 %s: error: {message}
-""" % (self.progname, self.progname)
+""" % (self.progname, self.progname, self.progname)
 
     @mock.patch('sys.stdout', new_callable=six.StringIO)
     @mock.patch('koji_cli.commands.activate_session')
@@ -204,7 +204,7 @@ The first option is the build target, not to be confused with the destination
 tag (where the build eventually lands) or build tag (where the buildroot
 contents are pulled from).
 
-You can list all available build targets using the 'koji list-targets' command.
+You can list all available build targets using the '%s list-targets' command.
 More detail can be found in the documentation.
 https://docs.pagure.org/koji/HOWTO/#package-organization
 (Specify the --help global option for a list of other help options)
@@ -225,7 +225,7 @@ Options:
   --repo-id=REPO_ID     Use a specific repo
   --noprogress          Do not display progress of the upload
   --background          Run the build at a lower priority
-""" % progname
+""" % (progname, progname)
         expected_stderr = ''
         self.assertMultiLineEqual(actual_stdout, expected_stdout)
         self.assertMultiLineEqual(actual_stderr, expected_stderr)

