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

add --only-free and --help options to tests #1161

Merged
merged 1 commit into from
Aug 20, 2023

Conversation

pinobatch
Copy link
Member

Use getopt(1) to provide -h, --help, and --dfsg options in test/fetch-test-deps.sh and test/run-tests.sh

  • -h or --help prints a usage message
  • --dfsg (which stands for Debian Free Software Guidelines) skips repositories with non-free licenses (pokecrystal and pokered) in favor of ucity to support packaging RGBDS for Debian
  • --dfsg can be combined with --get-hash or --get-paths

Fix #1160

Copy link
Member

@ISSOtm ISSOtm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the change, I think it's a good idea.

test/fetch-test-deps.sh Outdated Show resolved Hide resolved
test/run-tests.sh Outdated Show resolved Hide resolved
@Rangi42 Rangi42 added enhancement Typically new features; lesser priority than bugs meta This isn't related to the tools directly: repo organization, maintainership... labels Aug 17, 2023
@pinobatch pinobatch requested review from Rangi42 and ISSOtm August 17, 2023 22:20
test/fetch-test-deps.sh Outdated Show resolved Hide resolved
test/run-tests.sh Outdated Show resolved Hide resolved
test/run-tests.sh Outdated Show resolved Hide resolved
test/run-tests.sh Outdated Show resolved Hide resolved
test/run-tests.sh Outdated Show resolved Hide resolved
@pinobatch pinobatch requested a review from ISSOtm August 18, 2023 01:12
ISSOtm
ISSOtm previously requested changes Aug 18, 2023
test/fetch-test-deps.sh Outdated Show resolved Hide resolved
test/fetch-test-deps.sh Outdated Show resolved Hide resolved
test/fetch-test-deps.sh Outdated Show resolved Hide resolved
@pinobatch pinobatch requested a review from ISSOtm August 18, 2023 11:09
test/fetch-test-deps.sh Outdated Show resolved Hide resolved
test/fetch-test-deps.sh Outdated Show resolved Hide resolved
test/fetch-test-deps.sh Outdated Show resolved Hide resolved
test/run-tests.sh Outdated Show resolved Hide resolved
@Rangi42 Rangi42 changed the title add -h and --dfsg options to tests add --only-free and --help options to tests Aug 18, 2023
@pinobatch pinobatch requested a review from Rangi42 August 19, 2023 12:23
test/fetch-test-deps.sh Outdated Show resolved Hide resolved
test/fetch-test-deps.sh Outdated Show resolved Hide resolved
test/run-tests.sh Outdated Show resolved Hide resolved
test/run-tests.sh Outdated Show resolved Hide resolved
@pinobatch pinobatch requested a review from Rangi42 August 19, 2023 14:54
@Rangi42
Copy link
Contributor

Rangi42 commented Aug 19, 2023

Looks good to me! Again, just needs to squash the commits to get CI to pass.

@Rangi42 Rangi42 dismissed ISSOtm’s stale review August 19, 2023 16:19

Addressed review comments

@aaaaaa123456789
Copy link
Member

Bikeshedding time: if --no-nonfree is confusing because of the double negative, how about --skip-nonfree?

Copy link
Member

@ISSOtm ISSOtm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only thing I'd add is I support ax6's suggestion above... but eh let's just go with this flag name. It's good enough, I think.

Use getopt(1) to provide -h, --help, and --dfsg options in
test/fetch-test-deps.sh and test/run-tests.sh

- -h or --help prints a usage message
- --dfsg skips repositories with non-free licenses (pokecrystal
  and pokered) in favor of ucity to support packaging RGBDS for
  Debian
- --dfsg can be combined with --get-hash or --get-paths

change --dfsg option to --only-free

I was told the `+dfsg.1` used in Debian's repacks of upstream repos
and tarballs that contain non-free software is too obscure.  ISSOtm
and Rangi42 suggested `--only-free` which carries no Debian baggage.
https://www.reddit.com/r/debian/comments/66094l/what_is_dfsg_in_package_version_numbers/

- change --dfsg option to --only-free
- temporarily enable logging (set -x) to troubleshoot build failure
  on macOS

don't depend on util-linux getopt

util-linux getopt has long option parsing functionality not
available in macOS getopt.  Instead, parse the options in pure
Bash, treating each option as a separate word.

remove tracing from test

The removal of getopt indeed fixed test scripts on macOS.
This means we no longer need set -x to trace execution.

Co-authored-by: Eldred Habert <[email protected]>

use true/false for booleans in test scripts

I haven't had a chance to read the `bash(1)` manual cover to cover
(reading 6600 lines in `less(1)` is an attention challenge to me)
but it appears `true` and `false` are preferred because `true(1)`
and `false(1)` run faster than `test(1)`, to which `[` is a link.

Co-authored-by: Eldred Habert <[email protected]>

act on stylistic nitpicks by Rangi42

- remove explanation of `set -e` even from run-tests.sh, which had
  previously included one
- visually distinguish explanations of sections of a script from
  explanations of individual functions within a section
- change variable names that refer to script names to match the
  capitalized default filename of the script minus any extension
- change variable names that do not refer to script names to
  lowercase

Apply suggestions from code review

Co-authored-by: Rangi <[email protected]>

remane with_nonfree variable in tests to nonfree

per @Rangi42's preference
@ISSOtm
Copy link
Member

ISSOtm commented Aug 20, 2023

Rebased on top of current master to pull in CI fixes.

@aaaaaa123456789
Copy link
Member

Might be a good chance to fix the commit title and message.

@Rangi42 Rangi42 merged commit 7b3a05e into gbdev:master Aug 20, 2023
@pinobatch pinobatch deleted the test-dfsg branch August 23, 2023 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Typically new features; lesser priority than bugs meta This isn't related to the tools directly: repo organization, maintainership...
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Define a free subset of the test suite
5 participants