#2071 Better help for build/latest-build
Merged 2 years ago by tkopecek. Opened 2 years ago by tkopecek.
tkopecek/koji issue1808  into  master

file modified
+27 -3
@@ -11,6 +11,7 @@ 

  import re

  import stat

  import sys

+ import textwrap

  import time

  import traceback

  from collections import OrderedDict, defaultdict
@@ -452,7 +453,19 @@ 

  

  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>

+ 

+         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 '%prog list-targets' command.

+         More detail 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))

      parser.add_option("--skip-tag", action="store_true",

                        help=_("Do not attempt to tag package"))
@@ -2325,8 +2338,19 @@ 

  

  

  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> ...]

+ 

+         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))

      parser.add_option("--arch", help=_("List all of the latest packages for this arch"))

      parser.add_option("--all", action="store_true",

file modified
+29 -31
@@ -3,14 +3,11 @@ 

  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,19 @@ 

          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>

+ 

+ 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 '%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)

  

      @mock.patch('sys.stdout', new_callable=six.StringIO)

      @mock.patch('koji_cli.commands.activate_session')
@@ -147,11 +157,7 @@ 

          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 +199,14 @@ 

          actual_stdout = stdout.getvalue()

          actual_stderr = stderr.getvalue()

          expected_stdout = """Usage: %s build [options] <target> <srpm path or scm url>

+ 

+ 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 '%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)

  

  Options:
@@ -211,7 +225,7 @@ 

    --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)
@@ -257,11 +271,7 @@ 

          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 +362,7 @@ 

          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 +409,7 @@ 

          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 +456,7 @@ 

          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)

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

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

Metadata Update from @tkopecek:
- Pull-request tagged with: testing-ready

2 years ago

Metadata Update from @tkopecek:
- Pull-request untagged with: testing-ready
- Pull-request tagged with: doc, no_qe

2 years ago

There are some stray double quotes left in the string. I'm seeing this:

$ koji build --help
Usage: lkoji 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:

I'd probably avoid the nested parenthetical expressions and adjust the grammar a little. Maybe...

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. More detail can be found in the documentation.
https://docs.pagure.org/koji/HOWTO/#package-organization

It seems a little odd to format this usage string to match the code where it lies, and then transform it at run time. Also, the lines end up looking short because they are wrapped before col 80 in the code and then dedented by 8.

1 new commit added

  • fix grammar
2 years ago

Any other idea, how to make these strings "nicer"? I've added a commit with grammar fixes and formatted string for larger width. Not sure, if using it without dedent is more readable.

1 new commit added

  • fix test
2 years ago

Things look better with the adjusted width. I guess dedent might be the least awkward option at this point.

I put a few grammar adjustments here:
https://github.com/mikem23/koji-playground/commits/pagure/pr/2071

1 new commit added

  • more grammar changes
2 years ago

Commit ae05aa7 fixes this pull-request

Pull-Request has been merged by tkopecek

2 years ago