Repository: buildtesters/buildtest (language: python)

Issue:
[FEATURE]: add global option for `--pager`
### Please describe your feature request

currently we have `--pager` option as a sub-command for several commands including new features #1210 and #1211 for adding paging support for `buildtest history` and `buildtest inspect`.

Let's create a top-level option `buildtest --pager` and remove the options in each subcommand.

### Post question in Slack

- [X] I agree that I posted my question in [slack](https://hpcbuildtest.slack.com/) before creating this issue

### Is there an existing issue

- [X] I confirm there is no existing [issue](https://github.com/buildtesters/buildtest/issues) for this issue


Discussion / hints:
@jscook2345 following up on your recommendation in #1387 for `--row-count` we could probably do the same for `--pager` support. Does that make sense?

Currently we put options in each subcommand since option is not applicable for all subcommands but `--pager` is used with several commands so duplicating the option in each subcommand was more work and creating one option can help.

Fix patch:
diff --git a/bash_completion.sh b/bash_completion.sh
index 9e7b2b421..d19b2071a 100644
--- a/bash_completion.sh
+++ b/bash_completion.sh
@@ -195,7 +195,7 @@ _buildtest ()
       ;;
 
     report|rt)
-      local opts="--end --fail --filter --filterfields --format --formatfields --help --helpfilter --helpformat --latest --no-header --oldest --pager --pass --row-count --start --terse  -e -f -h -n -p -s -t c clear l list p path sm summary"
+      local opts="--end --fail --filter --filterfields --format --formatfields --help --helpfilter --helpformat --latest --no-header --oldest --pass --row-count --start --terse  -e -f -h -n -p -s -t c clear l list p path sm summary"
       COMPREPLY=( $( compgen -W "${opts}" -- $cur ) )
       case ${prev} in --color)
         COMPREPLY=( $( compgen -W "$(_supported_colors)" -- $cur ) )
@@ -241,7 +241,7 @@ _buildtest ()
           local opts="-h --help"
           COMPREPLY=( $( compgen -W "${opts}" -- $cur ) );;
         view|v)
-          local opts="--help --pager --theme -h -p -t"
+          local opts="--help --theme -h -p -t"
           COMPREPLY=( $( compgen -W "${opts}" -- $cur ) )
 
           case "${prev}" in --theme|-t)
@@ -305,7 +305,7 @@ _buildtest ()
            COMPREPLY=( $( compgen -W "${opts}" -- $cur ) );;
          # completion for rest of arguments
          *)
-           local longopts="--buildspec --count --executors --filter --filterfields --format --formatfields --group-by-executor --group-by-tags --help --helpfilter --helpformat --no-header --pager --paths --quiet --rebuild --row-count --tags --root --terse"
+           local longopts="--buildspec --count --executors --filter --filterfields --format --formatfields --group-by-executor --group-by-tags --help --helpfilter --helpformat --no-header --paths --quiet --rebuild --row-count --tags --root --terse"
            local shortopts="-b -e -h -n -p -q -r -t"
            local subcmds="invalid"
            local allopts="${longopts} ${shortopts} ${subcmds}"
@@ -329,8 +329,8 @@ _buildtest ()
          case ${COMP_WORDS[3]} in
          # completion for rest of arguments
          *)
-           local longopts="--pager"
-           local shortopts="-p"
+           local longopts="--help"
+           local shortopts="-h"
            local allopts="${longopts} ${shortopts}"
            COMPREPLY=( $( compgen -W "${allopts}" -- $cur ) );;
          esac
@@ -393,7 +393,7 @@ _buildtest ()
 
       case ${COMP_WORDS[2]} in
       list)
-        local opts="--help --no-header --pager --terse -h -n -t"
+        local opts="--help --no-header --terse -h -n -t"
         COMPREPLY=( $( compgen -W "${opts}" -- $cur ) )
 
         if [[ "${prev}" == "--color" ]]; then
@@ -446,7 +446,7 @@ _buildtest ()
     *)
       local cmds="build buildspec cd cdash clean config debugreport docs help info inspect history path report schema schemadocs stats stylecheck tutorial-examples unittests"
       local alias_cmds="bd bc cg debug it h hy rt style test"
-      local opts="--color --config --debug --editor --help --helpcolor --logpath --loglevel --print-log --no-color --report --version --view-log -c -d -h -l -r -V"
+      local opts="--color --config --debug --editor --help --helpcolor --logpath --loglevel --print-log --no-color --pager --report --version --view-log -c -d -h -l -p -r -V"
 
       case "${cur}" in
       # print main options to buildtest
diff --git a/docs/command_line_tutorial.rst b/docs/command_line_tutorial.rst
index 0fcd7e31d..060759f83 100644
--- a/docs/command_line_tutorial.rst
+++ b/docs/command_line_tutorial.rst
@@ -95,7 +95,7 @@ The ``buildtest rt summary`` can be useful if you want to summary of report file
 Buildtest supports paging support with ``buildtest rt`` which can be useful when you
 have lots of tests. To enable pagination you can run::
 
-    buildtest rt --pager
+    buildtest --pager rt
 
 Finally we can filter test records and format table columns via ``--filter`` and ``--format`` option. Let's try
 running the following command
@@ -257,7 +257,7 @@ We have an alias ``buildtest cg`` for **buildtest config** command. If you want
 
     .. command-output:: buildtest cg view
 
-We also support color themes (``buildtest cg view --theme <theme>``) and paging ``buildtest cg view --pager``.
+We also support color themes (``buildtest cg view --theme <theme>``) when showing content of buildtest configuration.
 
 Buildtest configuration file defines one or more :ref:`executors <configuring_executors>` that are used when
 writing test. Every test must be run by an executor. To retrieve all executors in a flat-listing you can run::


Test patch:
diff --git a/buildtest/cli/__init__.py b/buildtest/cli/__init__.py
index 048a76dd4..30ae8573e 100644
--- a/buildtest/cli/__init__.py
+++ b/buildtest/cli/__init__.py
@@ -221,6 +221,9 @@ def get_parser():
         action="store_true",
         help="Print available color options in a table format.",
     )
+    parser.add_argument(
+        "-p", "--pager", action="store_true", help="Enable PAGING when viewing result"
+    )
     parser.add_argument("-r", "--report", help="Specify path to test report file")
 
     subparsers = parser.add_subparsers(title="COMMANDS", dest="subcommands", metavar="")
@@ -415,9 +418,6 @@ def history_menu(subparsers):
         action="store_true",
         help="Print output in machine readable format",
     )
-    list_parser.add_argument(
-        "--pager", action="store_true", help="Enabling PAGING when viewing result"
-    )
 
     query = history_subparser.add_parser(
         "query", help="Query information for a particular build"
@@ -767,9 +767,6 @@ def buildspec_menu(subparsers):
         action="store_true",
         help="Print total count of records from the table.",
     )
-    buildspec_find.add_argument(
-        "--pager", action="store_true", help="Enable PAGING when viewing result"
-    )
     buildspec_find.add_argument(
         "-r",
         "--rebuild",
@@ -825,13 +822,9 @@ def buildspec_menu(subparsers):
     )
 
     # buildtest buildspec summary
-    buildspec_summary = subparsers_buildspec.add_parser(
+    subparsers_buildspec.add_parser(
         "summary", aliases=["sm"], help="Print summary of buildspec cache"
     )
-    # buildtest buildspec summary options
-    buildspec_summary.add_argument(
-        "-p", "--pager", action="store_true", help="Enable PAGING when viewing result"
-    )
     # buildtest buildspec validate
     buildspec_validate = subparsers_buildspec.add_parser(
         "validate", aliases=["val"], help="Validate buildspecs with JSON Schema"
@@ -913,9 +906,6 @@ def config_menu(subparsers):
         help="Specify a color theme, Pygments style to use when displaying output. See https://pygments.org/docs/styles/#getting-a-list-of-available-styles for available themes",
         choices=list(STYLE_MAP.keys()),
     )
-    view_parser.add_argument(
-        "-p", "--pager", action="store_true", help="Enable PAGING when viewing result"
-    )
     executor_group = executors.add_mutually_exclusive_group()
 
     # buildtest config executors
@@ -1095,9 +1085,6 @@ def report_menu(subparsers):
         action="store_true",
         help="Print total count of records from the table.",
     )
-    parser_report.add_argument(
-        "--pager", action="store_true", help="Enable PAGING when viewing result"
-    )
     parser_report_summary.add_argument(
         "--detailed", "-d", action="store_true", help="Enable a more detailed report"
     )
diff --git a/buildtest/cli/help.py b/buildtest/cli/help.py
index f0c76630d..169826e8d 100644
--- a/buildtest/cli/help.py
+++ b/buildtest/cli/help.py
@@ -102,6 +102,9 @@ def print_buildspec_help():
         "Discover and validate all buildspecs and load all validated buildspecs in cache",
     )
     table.add_row("buildtest buildspec find --rebuild", "Rebuild cache file")
+    table.add_row(
+        "buildtest --pager buildspec find", "Paginate output of buildspec cache"
+    )
     table.add_row(
         "buildtest buildspec find --root /tmp --rebuild",
         "Discover buildspecs in /tmp and rebuild buildspec cache",
@@ -183,7 +186,8 @@ def print_buildspec_help():
 
     table.add_row("buildtest buildspec summary", "Show summary of buildspec cache file")
     table.add_row(
-        "buildtest buildspec summary --pager", "Pageants the output of summary"
+        "buildtest --pager buildspec summary",
+        "Paginate the output of summary for buildspec cache",
     )
     table.add_row(
         "buildtest buildspec show python_hello",
@@ -237,6 +241,9 @@ def print_config_help():
     table.add_column("Description", justify="left", style="magenta")
 
     table.add_row("buildtest config view", "View content of configuration file")
+    table.add_row(
+        "buildtest --pager config view", "Paginate output of configuration file"
+    )
     table.add_row(
         "buildtest config validate", "Validate configuration file with JSON schema"
     )
@@ -336,6 +343,7 @@ def print_report_help():
     table.add_column("Description", justify="left", style="magenta")
 
     table.add_row("buildtest report", "Display all test results")
+    table.add_row("buildtest --pager report", "Paginate output of test results")
     table.add_row(
         "buildtest report --filter returncode=0", "Filter test results by returncode=0"
     )
@@ -378,6 +386,9 @@ def print_report_help():
     table.add_row(
         "buildtest report summary --detailed", "Show detailed summary of test report"
     )
+    table.add_row(
+        "buildtest --pager report summary", "Paginate output of report summary"
+    )
     console.print(table)
 
 
diff --git a/tests/cli/test_buildspec.py b/tests/cli/test_buildspec.py
index 46f627d5d..01e67f69a 100644
--- a/tests/cli/test_buildspec.py
+++ b/tests/cli/test_buildspec.py
@@ -95,19 +95,19 @@ def test_func_buildspec_find():
     # buildtest buildspec find --group-by-tags
     cache.print_by_tags()
 
-    # implements buildtest buildspec find --helpfilter
+    # buildtest buildspec find --helpfilter
     cache.print_filter_fields()
 
-    # implements buildtest buildspec find --helpformat
+    # buildtest buildspec find --helpformat
     cache.print_format_fields()
 
-    # implements buildtest buildspec find --filterfields
+    # buildtest buildspec find --filterfields
     cache.print_raw_filter_fields()
 
-    # implements buildtest buildspec find --formatfields
+    # buildtest buildspec find --formatfields
     cache.print_raw_format_fields()
 
-    # buildtest buildspec find --pager
+    # buildtest --pager buildspec find
     cache = BuildspecCache(configuration=configuration, pager=True)
     cache.print_tags()
     cache.print_buildspecfiles()
@@ -134,17 +134,28 @@ def test_func_buildspec_find():
     # buildtest buildspec find --row-count --executors
     cache.print_executors(row_count=True)
 
-    # test buildspec cache with color 'blue'
+    # test all commands with color 'blue'
+    # buildtest --color blue buildspec find --rebuild
     cache = BuildspecCache(rebuild=True, configuration=configuration, color="blue")
+    # buildtest --color blue buildspec find
     cache.print_buildspecs()
+    # buildtest --color blue buildspec find -b
     cache.print_buildspecfiles()
+    # buildtest --color blue buildspec find -e
     cache.print_executors()
+    # buildtest --color blue buildspec find -t
     cache.print_tags()
+    # buildtest --color blue buildspec find --group-by-executor
     cache.print_by_executors()
+    # buildtest --color blue buildspec find --group-by-tags
     cache.print_by_tags()
+    # buildtest --color blue buildspec find --helpfilter
     cache.print_filter_fields()
+    # buildtest --color blue buildspec find --helpformat
     cache.print_format_fields()
+    # buildtest --color blue buildspec find --filterfields
     cache.print_raw_filter_fields()
+    # buildtest --color blue buildspec find --formatfields
     cache.print_raw_format_fields()
 
 
@@ -239,11 +250,11 @@ def test_edit_file():
 
 @pytest.mark.cli
 def test_buildspec_find_filter():
-    # testing buildtest buildspec find --filter tags=fail
+    # buildtest buildspec find --filter tags=fail
     cache = BuildspecCache(filterfields={"tags": "fail"}, configuration=configuration)
     cache.print_buildspecs()
 
-    # testing buildtest buildspec find --filter buildspec=$BUILDTEST_ROOT/tutorials/hello_world.yml
+    # buildtest buildspec find --filter buildspec=$BUILDTEST_ROOT/tutorials/hello_world.yml
     cache = BuildspecCache(
         filterfields={
             "buildspec": os.path.join(BUILDTEST_ROOT, "tutorials", "hello_world.yml")
@@ -252,7 +263,7 @@ def test_buildspec_find_filter():
     )
     cache.print_buildspecs()
 
-    # testing buildtest buildspec find --filter type=script,executor=generic.local.sh,tags=fail
+    # buildtest buildspec find --filter type=script,executor=generic.local.sh,tags=fail
     cache = BuildspecCache(
         filterfields={
             "type": "script",
@@ -280,7 +291,7 @@ def test_buildspec_find_filter():
         # if we specify a directory path for buildspec filter this will raise an exception.
         BuildspecCache(filterfields={"buildspec": tf.name}, configuration=configuration)
 
-    # testing buildtest buildspec find --filter key1=val1,key2=val2
+    # buildtest buildspec find --filter key1=val1,key2=val2
     with pytest.raises(BuildTestError):
         cache = BuildspecCache(
             filterfields={"key1": "val1", "key2": "val2"}, configuration=configuration
@@ -290,7 +301,7 @@ def test_buildspec_find_filter():
 
 @pytest.mark.cli
 def test_buildspec_find_format():
-    # testing buildtest buildspec find --format name,type,tags,executor,description,buildspec
+    # buildtest buildspec find --format name,type,tags,executor,description,buildspec
     cache = BuildspecCache(
         formatfields="name,type,tags,executor,description,buildspec",
         configuration=configuration,
@@ -299,7 +310,7 @@ def test_buildspec_find_format():
 
     # Any invalid format fields will raise an exception of type BuildTestError
     with pytest.raises(BuildTestError):
-        # testing buildtest buildspec find --format field1
+        # buildtest buildspec find --format field1
         cache = BuildspecCache(formatfields="field1", configuration=configuration)
         cache.print_buildspecs()
 
@@ -310,10 +321,10 @@ def test_buildspec_find_roots():
         os.path.join(BUILDTEST_ROOT, "tests", "buildsystem"),
         os.path.join(BUILDTEST_ROOT, "tutorials"),
     ]
-    # testing buildtest buildspec find --root $BUILDTEST_ROOT/tests/buildsystem --root $BUILDTEST_ROOT/tutorials
+    # buildtest buildspec find --root $BUILDTEST_ROOT/tests/buildsystem --root $BUILDTEST_ROOT/tutorials
     BuildspecCache(roots=root_buildspecs, configuration=configuration)
 
-    # running buildtest buildspec find --root $BUILDTEST_ROOT/README.rst --root $BUILDTEST_ROOT/environment.yml
+    # buildtest buildspec find --root $BUILDTEST_ROOT/README.rst --root $BUILDTEST_ROOT/environment.yml
     BuildspecCache(
         roots=[
             os.path.join(BUILDTEST_ROOT, "README.rst"),
@@ -325,10 +336,11 @@ def test_buildspec_find_roots():
 
 @pytest.mark.cli
 def test_buildspec_summary():
-    # test buildtest buildspec summary
+    # buildtest buildspec summary
     summarize_buildspec_cache(
         configuration=configuration, pager=False, color=Color.default().name
     )
+    # buildtest --pager buildspec summary
     summarize_buildspec_cache(configuration=configuration, pager=True)
 
 
@@ -338,10 +350,10 @@ def test_buildspec_show():
 
     test_name = cache.get_random_tests(num_items=1)
 
-    # run buildtest buildspec show <test>
+    # buildtest buildspec show <test>
     show_buildspecs(test_name, configuration)
 
-    # run buildtest buildspec <test> show --theme monokai
+    # buildtest buildspec <test> show --theme monokai
     show_buildspecs(test_name, configuration, theme="monokai")
 
     # testing invalid buildspec name, it should not raise exception
@@ -365,7 +377,7 @@ def test_buildspec_show_fail():
     report = Report()
     # get a random failed test from report file to be used for showing content of buildspec file
     fail_tests = random.sample(report.get_test_by_state(state="FAIL"), 1)
-    # running buildtest buildspec show-fail <test> --theme monokai
+    # buildtest buildspec show-fail <test> --theme monokai
     show_failed_buildspecs(
         configuration=configuration, test_names=fail_tests, theme="monokai"
     )
diff --git a/tests/cli/test_config.py b/tests/cli/test_config.py
index 307d52227..78918f8ea 100644
--- a/tests/cli/test_config.py
+++ b/tests/cli/test_config.py
@@ -40,10 +40,10 @@ def test_config_systems():
 @pytest.mark.cli
 def test_view_configuration():
     view_configuration(configuration)
-    # testing -  buildtest config view --theme emacs
+    # buildtest config view --theme emacs
     view_configuration(configuration, theme="emacs")
 
-    # testing - buildtest config view --pager
+    # buildtest --pager config view
     view_configuration(configuration, pager=True)
 
 
@@ -69,7 +69,7 @@ def test_config_path():
 def test_config_executors():
     buildexecutor = BuildExecutor(configuration)
 
-    # run buildtest config executors --json
+    # buildtest config executors --json
     view_executors(
         configuration=configuration,
         buildexecutor=buildexecutor,
@@ -79,7 +79,7 @@ def test_config_executors():
         invalid=False,
     )
 
-    # run buildtest config executors --yaml
+    # buildtest config executors --yaml
     view_executors(
         configuration=configuration,
         buildexecutor=buildexecutor,
@@ -89,7 +89,7 @@ def test_config_executors():
         invalid=False,
     )
 
-    # run buildtest config executors -d
+    # buildtest config executors -d
     view_executors(
         configuration=configuration,
         buildexecutor=buildexecutor,
@@ -99,7 +99,7 @@ def test_config_executors():
         invalid=False,
     )
 
-    # run buildtest config executors -i
+    # buildtest config executors -i
     view_executors(
         configuration=configuration,
         buildexecutor=buildexecutor,
@@ -109,7 +109,7 @@ def test_config_executors():
         invalid=True,
     )
 
-    # run buildtest config executors
+    # buildtest config executors
     view_executors(
         configuration=configuration,
         buildexecutor=buildexecutor,
@@ -130,7 +130,7 @@ def test_disabled_invalid_executors():
 
     print("reading config file:", configfile)
     be = BuildExecutor(configuration)
-    # run buildtest config executors
+    # buildtest config executors -d
     view_executors(
         configuration=configuration,
         buildexecutor=be,
@@ -140,7 +140,7 @@ def test_disabled_invalid_executors():
         invalid=False,
     )
 
-    # run buildtest config executors
+    # buildtest config executors -i
     view_executors(
         configuration=configuration,
         buildexecutor=be,
diff --git a/tests/cli/test_report.py b/tests/cli/test_report.py
index 35b27ae09..8cfe3e9a3 100644
--- a/tests/cli/test_report.py
+++ b/tests/cli/test_report.py
@@ -201,12 +201,15 @@ def test_report_summary():
     report = Report()
     report_summary(report)
 
+    # buildtest --pager rt summary
     report = Report(pager=True)
     report_summary(report)
 
+    # buildtest rt summary --detailed
     report = Report()
     report_summary(report, detailed=True)
 
+    # buildtest --pager rt summary --detailed
     report = Report(pager=True)
     report_summary(report, detailed=True)
 
@@ -219,24 +222,18 @@ def test_report_summary():
     )
     report_summary(report, detailed=True)
 
+    # buildtest --color light_pink1 rt sm --detailed
     report = Report(color="light_pink1")
     report_summary(report, detailed=True)
 
-    report = Report(pager=True, color="light_pink1")
-    report_summary(report)
-
-    report = Report(color="BAD_COLOR")
-    report_summary(report)
-
-    report = Report(pager=True, color="BAD_COLOR")
+    # buildtest --pager rt sm --detailed
+    report = Report(pager=True)
     report_summary(report, detailed=True)
 
+    # buildtest --color BAD_COLOR rt sm --detailed
     report = Report(color="BAD_COLOR")
     report_summary(report, detailed=True)
 
-    report = Report(pager=True, color="BAD_COLOR")
-    report_summary(report)
-
 
 @pytest.mark.cli
 def test_report_list():