From 1a3f4e308e87775533c8eb9069370c42e29405c7 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Thu, 14 Nov 2024 04:53:47 -0500 Subject: [PATCH 1/3] Configure LF line endings for all shell scripts This adjustes `.gitattributes` to normalize line endings to LF (Unix style line endings) for all shell scripts in the project, not just shell scripts that are fixture scripts. Previously, shell scripts related to journey tests were not normalized this way. This is simpler to write, and `bash` and POSIX shell scripts are required to have LF line endings to be correct in general. On Windows, some `bash` builds (including the MSYS2 `bash` provided as part of the Git Bash environment) are patched to automatically accept Windows-style line endings, but I believe they are still considered technically incorrect. In practice, the effect of not using LF line endings when working with shell scripts on Windows may usually be where other tools are involved: ShellCheck, and at least some language servers for `bash` script editing, complain if CRLF line endings are used. --- .gitattributes | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.gitattributes b/.gitattributes index 8602f1fb954..cd5c10a80a7 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1,8 +1,8 @@ **/generated-archives/*.tar* filter=lfs-disabled diff=lfs merge=lfs -text # assure line feeds don't interfere with our working copy hash -**/tests/fixtures/**/*.sh text crlf=input eol=lf -/justfile text crlf=input eol=lf +*.sh text crlf=input eol=lf +justfile text crlf=input eol=lf # have GitHub include fixture-making scripts when it counts code **/tests/fixtures/**/*.sh linguist-vendored=false From bc6e4be0d529a7f372797580bccb02d1612495f5 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Thu, 14 Nov 2024 05:03:21 -0500 Subject: [PATCH 2/3] Don't use `with_program find` in journey tests This removes all uses of `with_program`, which is no longer used with any command other than `find`. The rationale is: - `find` is a standard command. Minimal systems may omit it, but they omit numerous other software that is needed for building and testing gitoxide. `find` is standardized and required by POSIX and is present on all common general-purpose installations of Unix-like operating systems. It is also present in the Git Bash environment (and most other MSYS2 environments) that may be used for running the test suite on Windows. - One purpose of `with_program` is to check for missing commands where, if not checked for, then tests might run but wrongly pass due to assertions that commands fail (where they should fail for a different reason). However, that does not seem to apply to any `with_program find`: all assertions with `find` in code to which that had been applied are using `expect_run_sh $SUCCESSFULLY`. `with_program` is typically applied in a `(` `)` subshell, which itself can have an important effect in isolating changes made within it. In this case, however, the code was either still enclosed equivalently, or appeared at the end of a larger `(` `)` scope. So this removes `(` `)` subshells that only existed to facilitate the deleted `with_program` uses and dedents. --- tests/journey/ein.sh | 244 +++++++++++++++++++++---------------------- tests/journey/gix.sh | 35 +++---- 2 files changed, 136 insertions(+), 143 deletions(-) diff --git a/tests/journey/ein.sh b/tests/journey/ein.sh index 408256e5178..89bffe1eed5 100644 --- a/tests/journey/ein.sh +++ b/tests/journey/ein.sh @@ -28,144 +28,142 @@ title "Porcelain ${kind}" ) ) snapshot="$snapshot/porcelain" - (with_program find - (when "using the 'tool' subcommand" - title "ein tool" - (with "a repo with a tiny commit history" - (small-repo-in-sandbox - title "ein tool estimate-hours" - (when "running 'estimate-hours'" - snapshot="$snapshot/estimate-hours" - (with "no arguments" - it "succeeds and prints only a summary" && { - WITH_SNAPSHOT="$snapshot/no-args-success" \ - expect_run_sh $SUCCESSFULLY "$exe tool estimate-hours 2>/dev/null" - } - ) - (with "the show-pii argument" - it "succeeds and shows information identifying people before the summary" && { - WITH_SNAPSHOT="$snapshot/show-pii-success" \ - expect_run_sh $SUCCESSFULLY "$exe tool estimate-hours --show-pii 2>/dev/null" - } - ) - (with "the omit-unify-identities argument" - it "succeeds and doesn't show unified identities (in this case there is only one author anyway)" && { - WITH_SNAPSHOT="$snapshot/no-unify-identities-success" \ - expect_run_sh $SUCCESSFULLY "$exe t estimate-hours --omit-unify-identities 2>/dev/null" - } - ) - (with "the --file-stats argument" - it "succeeds and shows file statistics" && { - WITH_SNAPSHOT="$snapshot/file-stats-success" \ - expect_run_sh $SUCCESSFULLY "$exe tool estimate-hours --file-stats 2>/dev/null" - } - ) - (with "the --line-stats argument" - it "succeeds and shows line statistics" && { - WITH_SNAPSHOT="$snapshot/line-stats-success" \ - expect_run_sh $SUCCESSFULLY "$exe tool estimate-hours --line-stats 2>/dev/null" - } - ) - (with "all --stats arguments and pii" - it "succeeds and shows all statistics" && { - WITH_SNAPSHOT="$snapshot/all-stats-success" \ - expect_run_sh $SUCCESSFULLY "$exe tool estimate-hours -pfl 2>/dev/null" - } - ) - (with "a branch name that doesn't exist" - it "fails and shows a decent enough error message" && { - WITH_SNAPSHOT="$snapshot/invalid-branch-name-failure" \ - expect_run_sh $WITH_FAILURE "$exe -q t estimate-hours . foobar" - } - ) + (when "using the 'tool' subcommand" + title "ein tool" + (with "a repo with a tiny commit history" + (small-repo-in-sandbox + title "ein tool estimate-hours" + (when "running 'estimate-hours'" + snapshot="$snapshot/estimate-hours" + (with "no arguments" + it "succeeds and prints only a summary" && { + WITH_SNAPSHOT="$snapshot/no-args-success" \ + expect_run_sh $SUCCESSFULLY "$exe tool estimate-hours 2>/dev/null" + } + ) + (with "the show-pii argument" + it "succeeds and shows information identifying people before the summary" && { + WITH_SNAPSHOT="$snapshot/show-pii-success" \ + expect_run_sh $SUCCESSFULLY "$exe tool estimate-hours --show-pii 2>/dev/null" + } + ) + (with "the omit-unify-identities argument" + it "succeeds and doesn't show unified identities (in this case there is only one author anyway)" && { + WITH_SNAPSHOT="$snapshot/no-unify-identities-success" \ + expect_run_sh $SUCCESSFULLY "$exe t estimate-hours --omit-unify-identities 2>/dev/null" + } + ) + (with "the --file-stats argument" + it "succeeds and shows file statistics" && { + WITH_SNAPSHOT="$snapshot/file-stats-success" \ + expect_run_sh $SUCCESSFULLY "$exe tool estimate-hours --file-stats 2>/dev/null" + } + ) + (with "the --line-stats argument" + it "succeeds and shows line statistics" && { + WITH_SNAPSHOT="$snapshot/line-stats-success" \ + expect_run_sh $SUCCESSFULLY "$exe tool estimate-hours --line-stats 2>/dev/null" + } + ) + (with "all --stats arguments and pii" + it "succeeds and shows all statistics" && { + WITH_SNAPSHOT="$snapshot/all-stats-success" \ + expect_run_sh $SUCCESSFULLY "$exe tool estimate-hours -pfl 2>/dev/null" + } + ) + (with "a branch name that doesn't exist" + it "fails and shows a decent enough error message" && { + WITH_SNAPSHOT="$snapshot/invalid-branch-name-failure" \ + expect_run_sh $WITH_FAILURE "$exe -q t estimate-hours . foobar" + } ) ) ) - (with "a mix of repositories" - (sandbox - repo-with-remotes dir/one-origin origin https://example.com/one-origin - repo-with-remotes origin-and-fork origin https://example.com/origin-and-fork fork https://example.com/other/origin-and-fork - repo-with-remotes special-origin special-name https://example.com/special-origin - repo-with-remotes no-origin - repo-with-remotes a-non-bare-repo-with-extension.git origin https://example.com/a-repo-with-extension.git - snapshot="$snapshot/tool" + ) + (with "a mix of repositories" + (sandbox + repo-with-remotes dir/one-origin origin https://example.com/one-origin + repo-with-remotes origin-and-fork origin https://example.com/origin-and-fork fork https://example.com/other/origin-and-fork + repo-with-remotes special-origin special-name https://example.com/special-origin + repo-with-remotes no-origin + repo-with-remotes a-non-bare-repo-with-extension.git origin https://example.com/a-repo-with-extension.git + snapshot="$snapshot/tool" - title "ein tool find" - (when "running 'find'" - snapshot="$snapshot/find" - (with "no arguments" - it "succeeds and prints a list of repository work directories" && { - WITH_SNAPSHOT="$snapshot/no-args-success" \ - expect_run_sh $SUCCESSFULLY "$exe tool find 2>/dev/null" - } - ) + title "ein tool find" + (when "running 'find'" + snapshot="$snapshot/find" + (with "no arguments" + it "succeeds and prints a list of repository work directories" && { + WITH_SNAPSHOT="$snapshot/no-args-success" \ + expect_run_sh $SUCCESSFULLY "$exe tool find 2>/dev/null" + } ) - title "ein tool organize" - (when "running 'organize'" - snapshot="$snapshot/organize" - (with "no arguments" - it "succeeds and informs about the operations that it WOULD do" && { - WITH_SNAPSHOT="$snapshot/no-args-success" \ - expect_run_sh $SUCCESSFULLY "$exe tool organize 2>/dev/null" - } - - it "does not change the directory structure at all" && { - WITH_SNAPSHOT="$snapshot/initial-directory-structure" \ - expect_run_sh $SUCCESSFULLY 'find . -maxdepth 2 | sort' - } - ) + ) + title "ein tool organize" + (when "running 'organize'" + snapshot="$snapshot/organize" + (with "no arguments" + it "succeeds and informs about the operations that it WOULD do" && { + WITH_SNAPSHOT="$snapshot/no-args-success" \ + expect_run_sh $SUCCESSFULLY "$exe tool organize 2>/dev/null" + } - (with "--execute" - it "succeeds" && { - WITH_SNAPSHOT="$snapshot/execute-success" \ - expect_run_sh $SUCCESSFULLY "$exe tool organize --execute 2>/dev/null" - } + it "does not change the directory structure at all" && { + WITH_SNAPSHOT="$snapshot/initial-directory-structure" \ + expect_run_sh $SUCCESSFULLY 'find . -maxdepth 2 | sort' + } + ) - it "changes the directory structure" && { - WITH_SNAPSHOT="$snapshot/directory-structure-after-organize" \ - expect_run_sh $SUCCESSFULLY 'find . -maxdepth 2 | sort' - } - ) + (with "--execute" + it "succeeds" && { + WITH_SNAPSHOT="$snapshot/execute-success" \ + expect_run_sh $SUCCESSFULLY "$exe tool organize --execute 2>/dev/null" + } - (with "--execute again" - it "succeeds" && { - WITH_SNAPSHOT="$snapshot/execute-success" \ - expect_run_sh $SUCCESSFULLY "$exe tool organize --execute 2>/dev/null" - } + it "changes the directory structure" && { + WITH_SNAPSHOT="$snapshot/directory-structure-after-organize" \ + expect_run_sh $SUCCESSFULLY 'find . -maxdepth 2 | sort' + } + ) - it "does not alter the directory structure as these are already in place" && { - WITH_SNAPSHOT="$snapshot/directory-structure-after-organize" \ - expect_run_sh $SUCCESSFULLY 'find . -maxdepth 2 | sort' - } - ) + (with "--execute again" + it "succeeds" && { + WITH_SNAPSHOT="$snapshot/execute-success" \ + expect_run_sh $SUCCESSFULLY "$exe tool organize --execute 2>/dev/null" + } - (with "--execute with move into subdirectory of itself" - (cd example.com - rm -Rf a-repo-with-extension origin-and-fork - mv one-origin ../ - cd .. - rmdir example.com && mv one-origin example.com - ) - it "succeeds" && { - WITH_SNAPSHOT="$snapshot/execute-success-new-root" \ - expect_run_sh $SUCCESSFULLY "$exe tool organize --execute 2>/dev/null" - } + it "does not alter the directory structure as these are already in place" && { + WITH_SNAPSHOT="$snapshot/directory-structure-after-organize" \ + expect_run_sh $SUCCESSFULLY 'find . -maxdepth 2 | sort' + } + ) - it "does alter the directory structure as these are already in place" && { - WITH_SNAPSHOT="$snapshot/directory-structure-after-organize-to-new-root" \ - expect_run_sh $SUCCESSFULLY 'find . -maxdepth 2 -type d | sort' - } + (with "--execute with move into subdirectory of itself" + (cd example.com + rm -Rf a-repo-with-extension origin-and-fork + mv one-origin ../ + cd .. + rmdir example.com && mv one-origin example.com ) - ) - if test "$kind" != "max-pure"; then - (with "running with no further arguments" - it "succeeds and informs about possible operations" && { - WITH_SNAPSHOT="$snapshot/no-args-failure" \ - expect_run_sh $WITH_CLAP_FAILURE "$exe t" + it "succeeds" && { + WITH_SNAPSHOT="$snapshot/execute-success-new-root" \ + expect_run_sh $SUCCESSFULLY "$exe tool organize --execute 2>/dev/null" + } + + it "does alter the directory structure as these are already in place" && { + WITH_SNAPSHOT="$snapshot/directory-structure-after-organize-to-new-root" \ + expect_run_sh $SUCCESSFULLY 'find . -maxdepth 2 -type d | sort' } ) - fi ) + if test "$kind" != "max-pure"; then + (with "running with no further arguments" + it "succeeds and informs about possible operations" && { + WITH_SNAPSHOT="$snapshot/no-args-failure" \ + expect_run_sh $WITH_CLAP_FAILURE "$exe t" + } + ) + fi ) ) ) diff --git a/tests/journey/gix.sh b/tests/journey/gix.sh index 1745468c994..4251e0ec26f 100644 --- a/tests/journey/gix.sh +++ b/tests/journey/gix.sh @@ -549,20 +549,17 @@ title "gix commit-graph" expect_run $WITH_FAILURE test -e "${PACK_FILE}".idx } - (with_program find - - if test "$kind" = "small" ; then - suffix=miniz-oxide - elif test "$kind" = "max-pure"; then - suffix=miniz-oxide-max - else - suffix=zlib-ng - fi - it "creates all pack objects, but the broken ones" && { - WITH_SNAPSHOT="$snapshot/broken-with-objects-dir-skip-checks-success-tree-$suffix" \ - expect_run_sh $SUCCESSFULLY 'find . -type f | sort' - } - ) + if test "$kind" = "small" ; then + suffix=miniz-oxide + elif test "$kind" = "max-pure"; then + suffix=miniz-oxide-max + else + suffix=zlib-ng + fi + it "creates all pack objects, but the broken ones" && { + WITH_SNAPSHOT="$snapshot/broken-with-objects-dir-skip-checks-success-tree-$suffix" \ + expect_run_sh $SUCCESSFULLY 'find . -type f | sort' + } ) ) ) @@ -582,12 +579,10 @@ title "gix commit-graph" "${PACK_FILE}.pack" . } - (with_program find - it "creates all pack objects" && { - WITH_SNAPSHOT="$snapshot/with-objects-dir-success-tree" \ - expect_run_sh $SUCCESSFULLY 'find . -type f | sort' - } - ) + it "creates all pack objects" && { + WITH_SNAPSHOT="$snapshot/with-objects-dir-success-tree" \ + expect_run_sh $SUCCESSFULLY 'find . -type f | sort' + } ) ) ) From 05e176c687cddf9b165bbfc4cfffc03373053bfa Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Thu, 14 Nov 2024 05:13:17 -0500 Subject: [PATCH 3/3] Remove the `with_program` helper This removes the journey test helper function `with_program`, which now has zero uses. (This is done in its own commit to make it easy to revert or examine later if a use case for it ever arises in the future.) --- tests/utilities.sh | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/tests/utilities.sh b/tests/utilities.sh index 3a33946de6e..f1c598e7ad7 100644 --- a/tests/utilities.sh +++ b/tests/utilities.sh @@ -7,21 +7,6 @@ RED="$(tput setaf 1 2>/dev/null || echo -n '')" OFFSET=( ) STEP=" " -function with_program () { - local program="${1:?}" - hash "$program" &>/dev/null || { - function expect_run () { - echo 1>&2 "${WHITE} - skipped (missing program)" - } - function expect_run_sh () { - echo 1>&2 "${WHITE} - skipped (missing program)" - } - function expect_run_sh_no_pipefail () { - echo 1>&2 "${WHITE} - skipped (missing program)" - } - } -} - function on_ci () { [ -n "${CI-}" ] || { function expect_run () {