dspinellis/git-issue

test.sh changes files in-place

Closed this issue · 3 comments

Thank you for the effort, and my apologies for not being able to code much for it.

Currently, in ./test.sh, test 1 passes the first time then fails the second and subsequent times. This is because something modifies files in-place and does not completely clean up.

(Side note, related to #57, is that all other tests pass on the first and subsequent tests. They certainly take longer, but this consistent with everything else in GfW: the windows interface appears to make some operations rather slow. The test-1 failure is not unique to Windows, I can reproduce on ubuntu-16.04.)

(I include some verbose script output below, but frankly I am confident that the problem is somewhere around lines 305 or 406 of ./test.sh, since those are the only lines that reference the foo file (noted as an untracked file in git status). Further, it's likely in the first 10 or so tests, since I'm typically able to interrupt soon after test 1 but not so finely-controlled that I know which test is doing it.)

Steps

  1. Fresh clone, so that git status produces nothing.

  2. Run bash -x test.sh, interrupt after the first test.

    verbose test output
    $ bash -x test.sh
    ++ mktemp -d
    + TopDir=/tmp/tmp.iDdKpmIPqD
    + jq --version
    jq-1.6
    + curl --version
    curl 7.68.0 (x86_64-w64-mingw32) libcurl/7.68.0 OpenSSL/1.1.1d (Schannel) zlib/1.2.11 libidn2/2.3.0 nghttp2/1.40.0
    Release-Date: 2020-01-08
    Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtsp smtp smtps telnet tftp
    Features: AsynchDNS HTTP2 HTTPS-proxy IDN IPv6 Kerberos Largefile libz Metalink MultiSSL NTLM SPNEGO SSL SSPI TLS-SRP
    + echo 'Test artifacts saved in /tmp/tmp.iDdKpmIPqD'
    Test artifacts saved in /tmp/tmp.iDdKpmIPqD
    + command -v gdate
    + DATEBIN=date
    + '[' -n '' ']'
    + echo 'TAP version 13'
    TAP version 13
    + ntest=0
    ++ pwd
    + gi=/c/Users/r2/Projects/test/git-issue/git-issue.sh
    ++ echo /c/Users/r2/Projects/test/git-issue/git-issue.sh
    ++ sed 's/[^0-9A-Za-z]/\\&/g'
    + gi_re='\/c\/Users\/r2\/Projects\/test\/git\-issue\/git\-issue\.sh'
    ++ pwd
    + GIT_ISSUE_LIB_PATH=/c/Users/r2/Projects/test/git-issue/lib
    + export GIT_ISSUE_LIB_PATH
    + start sync-docs
    + ntest=1
    + testname=sync-docs
    + test -n sync-docs
    + echo 'Test 1: sync-docs'
    + GenFiles='git-issue.sh git-issue.1'
    + git diff --quiet HEAD
    + sh sync-docs.sh --no-user-agent
    ++ git status --porcelain -- 'git-issue.sh git-issue.1'
    + Status=
    + '[' -z '' ']'
    + ok 'make sync-docs left git-issue.sh git-issue.1 as committed'
    + message ok 'make sync-docs left git-issue.sh git-issue.1 as committed'
    + local okfail
    + okfail=ok
    + shift
    + '[' 'make sync-docs left git-issue.sh git-issue.1 as committed' ']'
    + echo 'ok 1 - make sync-docs left git-issue.sh git-issue.1 as committed'
    + sed 's/\/c\/Users\/r2\/Projects\/test\/git\-issue\/git\-issue\.sh/gi/'
    ok 1 - make sync-docs left git-issue.sh git-issue.1 as committed
    + cd /tmp/tmp.iDdKpmIPqD
  3. git status and notice that two files have changed.

    $ git status
    On branch master
    Your branch is up to date with 'origin/master'.
    
    Changes not staged for commit:
      (use "git add <file>..." to update what will be committed)
      (use "git restore <file>..." to discard changes in working directory)
            modified:   git-issue.1
            modified:   git-issue.sh
    
    Untracked files:
      (use "git add <file>..." to include in what will be committed)
            foo
    git diff
    diff --git a/git-issue.1 b/git-issue.1
    index 57ef847..0aa7095 100644
    --- a/git-issue.1
    +++ b/git-issue.1
    @@ -172,7 +172,7 @@ Run \fCcommand\fP in every issue directory. The following environment variables
    
       The command can read, add/remove or edit any of the issue's attributes.
       Some potentially useful scripts to be used with this command are in the scripts/ directory.
    -  Remember to inspect the results (e.g. \fCgi git diff\fP) and commit them with \fCgi git commit -a\fP.
    +  Remember to inspect the results (e.g \fCgi git diff\fP) and commit them with \fCgi git commit -a\fP.
    
     .RE
     .PP
    @@ -203,7 +203,7 @@ Import/update GitHub/GitLab issues from the specified project.
     .RS 4
     Create the issue in the provided GitHub repository.
       With the \fC-e\fP option any escape sequences for the attributes present in the description, will be replaced as above.
    -  This can be used to e.g. export an unsupported attribute to GitHub as text.
    +  This can be used to e.g export an unsupported attribute to GitHub as text.
     .RE
     .PP
     \fBgit issue export\fP
    @@ -211,7 +211,7 @@ Create the issue in the provided GitHub repository.
     Export issues for the specified project.
       Only the issues that have been imported and modified (or created by \fCgit issue create\fP) by \fCgit-issue\fP will be exported.
       With the \fC-e\fP option any escape sequences for the attributes present in the description, will be replaced as above.
    -  This can be used to e.g. export an unsupported attribute to GitHub as text.
    +  This can be used to e.g export an unsupported attribute to GitHub as text.
     .RE
     .PP
     \fBgit issue exportall\fP
    @@ -644,14 +644,14 @@ Export modified issues back to GitHub
     .ft C
     .nf
     $ git issue export github dspinellis git-issue-test-issues # Needs a token with the relevant permissions
    -Issue b83d92872dc16440402516a5f4ce1b8cc6436344 not modified, skipping...
    -Comment a93764f32179e93493ceb0a7060efce1e980aff1 not modified, skipping...
    +Issue b83d92872dc16440402516a5f4ce1b8cc6436344 hasn't been modified, skipping...
    +Comment a93764f32179e93493ceb0a7060efce1e980aff1 hasn't been modified, skipping...
     Exporting issue 9179d381135273220301f175c03b101b3e9c703d as #15
    -Issue 3651dd38e4e1d9dbce66649710324235c773fe78 not modified, skipping...
    +Issue 3651dd38e4e1d9dbce66649710324235c773fe78 hasn't been modified, skipping...
     Updating comment d72c68d0177b500a91ea37548e6594f84457fd5b...
    -Comment 6966d4d718c80cf8635e9276d6f391de70c22f93 not modified, skipping...
    -Comment 85293a6904d0fbd6238fbb2e1c36fc65af9ffc60 not modified, skipping...
    -Comment aea83723c0414ff135afcfb5165d64f8a7ad687c not modified, skipping...
    +Comment 6966d4d718c80cf8635e9276d6f391de70c22f93 hasn't been modified, skipping...
    +Comment 85293a6904d0fbd6238fbb2e1c36fc65af9ffc60 hasn't been modified, skipping...
    +Comment aea83723c0414ff135afcfb5165d64f8a7ad687c hasn't been modified, skipping...
    
     .fi
     .ft R
    diff --git a/git-issue.sh b/git-issue.sh
    index 7a1dcce..ebff85b 100755
    --- a/git-issue.sh
    +++ b/git-issue.sh
    @@ -1407,8 +1407,8 @@ Synchronize with remote repositories
        pull       Update local Git repository with remote changes
        import     Import/update GitHub/GitLab issues from the specified project
        create     Create the issue in the provided GitHub repository
    -   export     Export modified issues for the specified project
    -   exportall  Export all open issues in the database (-a to include closed ones) to GitHub/GitLab. Useful for cloning whole repositories
    +   export     Export issues for the specified project
    +   exportall  Export all open issues in the database (-a to include closed ones) to GitHub/GitLab Useful for cloning whole repositories
    
     Help and debug
        help       Display help information about git issue
  4. bash -x test.sh now fails test 1, because git diff returns some changed files.

BTW: GfW users will likely also need to know that more is used in the tests if not in git-issue.sh itself. more is not installed by default in GfW, so my workaround for testing has been

$ PAGER=cat ./test.sh

### or

$ export PAGER=cat # or PAGER=less
$ ./test.sh

Perhaps this is mostly my understanding: when test.sh says fail 1 - Uncommitted..., this to me indicates a compatibility problem with the platform being tested. As I read the tests more, this may be a known thing, since it does say "skipping".

Would skip 1 - Uncommitted... make more sense? It would be less of an alarm (to me, at least).

(I still wonder if it would be better to have an "undo sync-docs.sh" action in the test so that the test does not change things.)

I found (and hopefully corrected) the (somewhat complex) issue. The problem was an incorrect edit from my side, which wasn't detected, because the corresponding test wasn't working due to an earlier shellcheck warning fix.

I was just working on a PR for some of those (namely, the quoted $GenFiles plus one of the sed recipes. On ubuntu, test 1 now passes on consecutive runs. On windows in GfW it fails due to a CRLF thing only, no substantive difference. If I can work out exactly how to fix that cleanly and generally, I'll report back (as a PR). Otherwise, this issue is fixed. Thanks!