Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test/system: Various improvements to the 'help' tests #1386

Conversation

debarshiray
Copy link
Member

@debarshiray debarshiray commented Oct 10, 2023

No description provided.

debarshiray added a commit to debarshiray/toolbox that referenced this pull request Oct 10, 2023
@debarshiray debarshiray force-pushed the wip/rishi/test-system-help-groff-1.23-font-C branch from 1b4bc08 to dacecd8 Compare October 10, 2023 18:42
debarshiray added a commit to debarshiray/toolbox that referenced this pull request Oct 10, 2023
@debarshiray debarshiray force-pushed the wip/rishi/test-system-help-groff-1.23-font-C branch from dacecd8 to 16519d1 Compare October 10, 2023 18:49
@softwarefactory-project-zuul
Copy link

Build failed.
https://softwarefactory-project.io/zuul/t/local/buildset/836d327e412142db8edcc7970ae1af09

✔️ unit-test SUCCESS in 8m 29s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 31s
✔️ unit-test-restricted SUCCESS in 7m 54s
system-test-fedora-rawhide FAILURE in 35m 27s
system-test-fedora-39 FAILURE in 33m 30s
✔️ system-test-fedora-38 SUCCESS in 25m 15s
✔️ system-test-fedora-37 SUCCESS in 24m 01s

@debarshiray
Copy link
Member Author

recheck

@softwarefactory-project-zuul
Copy link

Build failed.
https://softwarefactory-project.io/zuul/t/local/buildset/6bfb94f5b1af4e88a9d464a4013e3770

✔️ unit-test SUCCESS in 8m 56s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 49s
✔️ unit-test-restricted SUCCESS in 7m 58s
system-test-fedora-rawhide FAILURE in 37m 02s
system-test-fedora-39 FAILURE in 36m 14s
✔️ system-test-fedora-38 SUCCESS in 29m 44s
✔️ system-test-fedora-37 SUCCESS in 29m 33s

debarshiray added a commit to debarshiray/toolbox that referenced this pull request Oct 11, 2023
Commit 5e63e9e added a 'help' command to show the toolbox(1)
manual or a manual page for a specific command, and made the --help flag
identical to it.  Therefore it's misleading to say that the --help flag
should show the usage screen.  The usage screen is a brief listing of
the commands and options, which isn't the same thing as the more
detailed manuals.

Fallout from b27795a

containers#1386
@softwarefactory-project-zuul
Copy link

Build failed.
https://softwarefactory-project.io/zuul/t/local/buildset/d54eab08427c45ecaf0e3e6596e51314

✔️ unit-test SUCCESS in 8m 45s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 4m 12s
✔️ unit-test-restricted SUCCESS in 7m 35s
system-test-fedora-rawhide FAILURE in 35m 46s
system-test-fedora-39 FAILURE in 36m 12s
✔️ system-test-fedora-38 SUCCESS in 29m 37s
✔️ system-test-fedora-37 SUCCESS in 28m 09s

debarshiray added a commit to debarshiray/toolbox that referenced this pull request Oct 11, 2023
Commit 5e63e9e added a 'help' command to show the toolbox(1)
manual or a manual page for a specific command, and made the --help flag
identical to it.  Therefore it's misleading to say that the --help flag
should show the usage screen.  The usage screen is a brief listing of
the commands and options, which isn't the same thing as the more
detailed manuals.

Later, after this test was written, commit 40fc168 added a
fallback for host operating systems without man(1), like Fedora CoreOS,
that would show a very brief usage screen with only the most common
commands.

To make it more confusing, the test was checking for a string that's
common to both the toolbox(1) manual and the fallback brief usage screen
that might be shown by 'toolbox --help'.  This meant that it was neither
able to distinguish between the code paths nor ensure that they were
working as intended.

This was resolved by adapting the existing 'toolbox --help' test to
strictly ensure that it's showing the toolbox(1) manual when man(1) is
present, and by adding a new test to strictly ensure that it's showing
the fallback brief usage screen when man(1) is absent.

Until Bats 1.10.0, 'run --keep-empty-lines' had a bug where it counted
the trailing newline on the last line as a separate line [1].  However,
Bats 1.10.0 is only available in Fedora >= 39 and is absent from Fedoras
37 and 38.

Fallout from b27795a

[1] Bats commit 6648e2143bffb933
    bats-core/bats-core@6648e2143bffb933
    bats-core/bats-core#708

containers#1386
@debarshiray debarshiray force-pushed the wip/rishi/test-system-help-groff-1.23-font-C branch from f34b2ee to 033eb34 Compare October 11, 2023 17:55
@softwarefactory-project-zuul
Copy link

Build failed.
https://softwarefactory-project.io/zuul/t/local/buildset/8b4173e99a0940aeb4a768fc4a091b97

✔️ unit-test SUCCESS in 8m 21s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 41s
✔️ unit-test-restricted SUCCESS in 7m 33s
system-test-fedora-rawhide FAILURE in 57m 06s
system-test-fedora-39 TIMED_OUT in 40m 23s
✔️ system-test-fedora-38 SUCCESS in 37m 23s
✔️ system-test-fedora-37 SUCCESS in 37m 23s

debarshiray added a commit to debarshiray/toolbox that referenced this pull request Oct 11, 2023
With the recent expansion of the test suite, it's necessary to increase
the timeout for all Fedora nodes to prevent the CI from timing out.

containers#1386
@softwarefactory-project-zuul
Copy link

Build failed.
https://softwarefactory-project.io/zuul/t/local/buildset/77311b140e4a4d7caca98c6a47c95296

✔️ unit-test SUCCESS in 8m 43s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 51s
✔️ unit-test-restricted SUCCESS in 7m 37s
system-test-fedora-rawhide NODE_FAILURE Node request 200-0006469544 failed in 0s
system-test-fedora-39 NODE_FAILURE Node request 200-0006469545 failed in 0s
✔️ system-test-fedora-38 SUCCESS in 31m 33s
✔️ system-test-fedora-37 SUCCESS in 31m 49s

@debarshiray
Copy link
Member Author

recheck

@softwarefactory-project-zuul
Copy link

Build failed.
https://softwarefactory-project.io/zuul/t/local/buildset/a0483ca0e06c4fbaaf8d0568769d0bc6

✔️ unit-test SUCCESS in 8m 35s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 53s
✔️ unit-test-restricted SUCCESS in 7m 46s
system-test-fedora-rawhide FAILURE in 38m 00s
system-test-fedora-39 FAILURE in 36m 54s
✔️ system-test-fedora-38 SUCCESS in 31m 38s
✔️ system-test-fedora-37 SUCCESS in 30m 59s

debarshiray added a commit to debarshiray/toolbox that referenced this pull request Oct 11, 2023
Currently, there's no way to get assert_line to use the stderr_lines
array [1].  This is worked around by assigning stderr_lines to the
'lines' array.

[1] bats-core/bats-assert#42

containers#1386
@softwarefactory-project-zuul
Copy link

Build failed.
https://softwarefactory-project.io/zuul/t/local/buildset/e9526d9379b2424f8d33ee3bdc3db9d3

✔️ unit-test SUCCESS in 8m 43s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 11s
✔️ unit-test-restricted SUCCESS in 7m 12s
system-test-fedora-rawhide FAILURE in 37m 23s
system-test-fedora-39 FAILURE in 36m 35s
✔️ system-test-fedora-38 SUCCESS in 31m 27s
✔️ system-test-fedora-37 SUCCESS in 31m 13s

@debarshiray
Copy link
Member Author

recheck

@softwarefactory-project-zuul
Copy link

Build failed.
https://softwarefactory-project.io/zuul/t/local/buildset/38c318184564438ea1d099237e985331

✔️ unit-test SUCCESS in 9m 15s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 20s
✔️ unit-test-restricted SUCCESS in 8m 02s
system-test-fedora-rawhide FAILURE in 36m 23s
system-test-fedora-39 FAILURE in 38m 30s
✔️ system-test-fedora-38 SUCCESS in 33m 47s
✔️ system-test-fedora-37 SUCCESS in 33m 15s

Until Bats 1.10.0, 'run --keep-empty-lines' had a bug where it counted
the trailing newline on the last line as a separate line [1].  However,
Bats 1.10.0 is only available in Fedora >= 39 and is absent from Fedoras
37 and 38.

[1] Bats commit 6648e2143bffb933
    bats-core/bats-core@6648e2143bffb933
    bats-core/bats-core#708

containers#1387
debarshiray added a commit to debarshiray/toolbox that referenced this pull request Oct 12, 2023
debarshiray added a commit to debarshiray/toolbox that referenced this pull request Oct 12, 2023
Commit 5e63e9e added a 'help' command to show the toolbox(1)
manual or a manual page for a specific command, and made the --help flag
identical to it.  Therefore it's misleading to say that the --help flag
should show the usage screen.  The usage screen is a brief listing of
the commands and options, which isn't the same thing as the more
detailed manuals.

Later, after this test was written, commit 40fc168 added a
fallback for host operating systems without man(1), like Fedora CoreOS,
that would show a very brief usage screen with only the most common
commands.

To make it more confusing, the test was checking for a string that's
common to both the toolbox(1) manual and the fallback brief usage screen
that might be shown by 'toolbox --help'.  This meant that it was neither
able to distinguish between the code paths nor ensure that they were
working as intended.

This was resolved by adapting the existing 'toolbox --help' test to
strictly ensure that it's showing the toolbox(1) manual when man(1) is
present, and by adding a new test to strictly ensure that it's showing
the fallback brief usage screen when man(1) is absent.

Until Bats 1.10.0, 'run --keep-empty-lines' had a bug where it counted
the trailing newline on the last line as a separate line [1].  However,
Bats 1.10.0 is only available in Fedora >= 39 and is absent from Fedoras
37 and 38.

Fallout from b27795a

[1] Bats commit 6648e2143bffb933
    bats-core/bats-core@6648e2143bffb933
    bats-core/bats-core#708

containers#1386
debarshiray added a commit to debarshiray/toolbox that referenced this pull request Oct 12, 2023
With the recent expansion of the test suite, it's necessary to increase
the timeout for all Fedora nodes to prevent the CI from timing out.

containers#1386
@debarshiray debarshiray force-pushed the wip/rishi/test-system-help-groff-1.23-font-C branch from 12c39ed to 3bd780b Compare October 12, 2023 09:20
debarshiray added a commit to debarshiray/toolbox that referenced this pull request Oct 12, 2023
Currently, there's no way to get assert_line to use the stderr_lines
array [1].  This is worked around by assigning stderr_lines to the
'lines' array.

[1] bats-core/bats-assert#42

containers#1386
@softwarefactory-project-zuul
Copy link

Build failed.
https://softwarefactory-project.io/zuul/t/local/buildset/bbc614681d164ad2876d000dfeb8a020

✔️ unit-test SUCCESS in 9m 10s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 4m 01s
✔️ unit-test-restricted SUCCESS in 8m 16s
system-test-fedora-rawhide FAILURE in 50m 01s
system-test-fedora-39 FAILURE in 44m 49s
✔️ system-test-fedora-38 SUCCESS in 42m 55s
✔️ system-test-fedora-37 SUCCESS in 42m 13s

With the recent expansion of the test suite, it's necessary to increase
the timeout for all Fedora nodes to prevent the CI from timing out.

containers#1387
Commit 5e63e9e added a 'help' command to show the toolbox(1)
manual or a manual page for a specific command, and made the --help flag
identical to it.  Therefore it's misleading to say that the --help flag
should show the usage screen.  The usage screen is a brief listing of
the commands and options, which isn't the same thing as the more
detailed manuals.

Later, after this test was written, commit 40fc168 added a
fallback for host operating systems without man(1), like Fedora CoreOS,
that would show a very brief usage screen with only the most common
commands.

To make it more confusing, the test was checking for a string that's
common to both the toolbox(1) manual and the fallback brief usage screen
that might be shown by 'toolbox --help'.  This meant that it was neither
able to distinguish between the code paths nor ensure that they were
working as intended.

This was resolved by adapting the existing 'toolbox --help' test to
strictly ensure that it's showing the toolbox(1) manual when man(1) is
present, and by adding a new test to strictly ensure that it's showing
the fallback brief usage screen when man(1) is absent.

Until Bats 1.10.0, 'run --keep-empty-lines' had a bug where it counted
the trailing newline on the last line as a separate line [1].  However,
Bats 1.10.0 is only available in Fedora >= 39 and is absent from Fedoras
37 and 38.

Fallout from b27795a

[1] Bats commit 6648e2143bffb933
    bats-core/bats-core@6648e2143bffb933
    bats-core/bats-core#708

containers#1386
Currently, there's no way to get assert_line to use the stderr_lines
array [1].  This is worked around by assigning stderr_lines to the
'lines' array.

[1] bats-core/bats-assert#42

containers#1386
@debarshiray debarshiray force-pushed the wip/rishi/test-system-help-groff-1.23-font-C branch from 3bd780b to e7f729f Compare October 12, 2023 11:18
... for both 'toolbox help' and 'toolbox --help'.

containers#1386
@softwarefactory-project-zuul
Copy link

Build failed.
https://softwarefactory-project.io/zuul/t/local/buildset/d89febff13124706b1f362d3c6aec3e2

✔️ unit-test SUCCESS in 8m 10s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 31s
✔️ unit-test-restricted SUCCESS in 7m 31s
system-test-fedora-rawhide FAILURE in 36m 20s
system-test-fedora-39 FAILURE in 35m 30s
✔️ system-test-fedora-38 SUCCESS in 30m 58s
✔️ system-test-fedora-37 SUCCESS in 29m 50s

Currently, some of the names of the tests were too long, and had
inconsistent and verbose wording.  This made it difficult to look at
them and get a gist of all the scenarios being tested.  The names are
like headings.  They shouldn't be too long, should capture the primary
objective of the test and be consistent in their wording.

Note that the term 'usage screen' was particularly confusing.  Prior to
commit 3dc106e, 'usage screen' in the names of the tests also
referred to the very brief listing of the commands and options that's
shown by 'toolbox help' and 'toolbox --help' in the absence of man(1).
In the context of this change, the term referred to the brief two line
error message that's shown when an unknown command or flag is used.  So,
it will be good to not use it anymore.

containers#1386
@softwarefactory-project-zuul
Copy link

Build failed.
https://softwarefactory-project.io/zuul/t/local/buildset/b62cca3eff7a4f0295b24eb17e140d8f

✔️ unit-test SUCCESS in 10m 13s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 4m 52s
✔️ unit-test-restricted SUCCESS in 8m 44s
system-test-fedora-rawhide FAILURE in 35m 53s
system-test-fedora-39 FAILURE in 42m 03s
✔️ system-test-fedora-38 SUCCESS in 35m 53s
✔️ system-test-fedora-37 SUCCESS in 34m 40s

@softwarefactory-project-zuul
Copy link

Build failed.
https://softwarefactory-project.io/zuul/t/local/buildset/4cfcd4a006ba4c34af2eac0a8e69f3a5

✔️ unit-test SUCCESS in 8m 38s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 27s
✔️ unit-test-restricted SUCCESS in 7m 49s
system-test-fedora-rawhide FAILURE in 36m 32s
system-test-fedora-39 FAILURE in 35m 43s
✔️ system-test-fedora-38 SUCCESS in 31m 31s
✔️ system-test-fedora-37 SUCCESS in 29m 52s

@debarshiray
Copy link
Member Author

recheck

@softwarefactory-project-zuul
Copy link

Build failed.
https://softwarefactory-project.io/zuul/t/local/buildset/4fc5f41184854828852bbceb7912e59c

✔️ unit-test SUCCESS in 8m 42s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 50s
✔️ unit-test-restricted SUCCESS in 7m 34s
system-test-fedora-rawhide FAILURE in 48m 36s
system-test-fedora-39 FAILURE in 41m 15s
✔️ system-test-fedora-38 SUCCESS in 35m 03s
✔️ system-test-fedora-37 SUCCESS in 37m 38s

Ansible's built-in 'package' module doesn't show any details when
installing the RPMs.  All that can be seen is:
  TASK [Install RPM packages]
  fedora-rawhide | changed

Therefore, there's no way to know what version of the packages got
installed.

In this case, not knowing the go-md2man(1) version being used by the CI
makes it difficult to know why the tests are failing on Fedora Rawhide
and Fedora 39 with:
  not ok 3 help: Command 'help' in 177ms
  # (from function `assert_line' in file
       test/system/libs/bats-assert/src/assert.bash, line 479,
  #  in test file test/system/002-help.bats, line 48)
  #   `assert_line --index 0 --partial "toolbox(1)"' failed
  # /usr/bin/man
  #
  # -- line does not contain substring --
  # index     : 0
  # substring : toolbox(1)
  # line      : troff:<standard input>:33: warning: cannot select font
                  'C'
  # --
  #

It could be either because the CI is still using an older version of
go-md2man(1) [1,2], or that there's some other problem.

[1] Fedora golang-github-cpuguy83-md2man commit 117806d50e401c19
    https://src.fedoraproject.org/rpms/golang-github-cpuguy83-md2man/c/117806d50e401c19
    https://src.fedoraproject.org/rpms/golang-github-cpuguy83-md2man/pull-request/3

[2] go-md2man commit d85280db9b54b574
    cpuguy83/go-md2man@d85280db9b54b574
    cpuguy83/go-md2man#99

containers#1386
@softwarefactory-project-zuul
Copy link

Build failed.
https://softwarefactory-project.io/zuul/t/local/buildset/d9945d61a3904ba49e6a50c32de75879

✔️ unit-test SUCCESS in 8m 48s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 33s
✔️ unit-test-restricted SUCCESS in 7m 51s
system-test-fedora-rawhide FAILURE in 36m 08s
system-test-fedora-39 FAILURE in 35m 16s
✔️ system-test-fedora-38 SUCCESS in 29m 19s
✔️ system-test-fedora-37 SUCCESS in 27m 49s

@debarshiray
Copy link
Member Author

This is interesting:

fedora-rawhide | not ok 3 help: Command 'help' in 222ms
fedora-rawhide | # (from function `assert_line' in file test/system/libs/bats-assert/src/assert.bash, line 479,
fedora-rawhide | #  in test file test/system/002-help.bats, line 50)
fedora-rawhide | #   `assert_line --index 3 --partial "toolbox - Tool for containerized command line environments on Linux"' failed
fedora-rawhide | # /usr/bin/man
fedora-rawhide | #
fedora-rawhide | # -- line does not contain substring --
fedora-rawhide | # index     : 3
fedora-rawhide | # substring : toolbox - Tool for containerized command line environments on Linux
fedora-rawhide | # line      :        toolbox ‐ Tool for containerized command line environments on Linux
fedora-rawhide | # --
fedora-rawhide | #

@debarshiray debarshiray merged commit cb4e5dd into containers:main Oct 18, 2023
2 checks passed
@debarshiray debarshiray deleted the wip/rishi/test-system-help-groff-1.23-font-C branch October 18, 2023 11:39
@debarshiray debarshiray changed the title test/system: Unbreak the 'help' tests on Fedora >= 39 with GNU roff 1.23 test/system: Various improvements to the 'help' tests Oct 18, 2023
@debarshiray
Copy link
Member Author

Those failures are because of some changes in GNU roff 1.23.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant