wjdp / htmltest

:white_check_mark: Test generated HTML for problems
MIT License
323 stars 54 forks source link

name mismatch between macOS release and godownloader.sh script #170

Closed bridgetkromhout closed 3 years ago

bridgetkromhout commented 3 years ago

Describe the bug

There's a subtle silent failure on macOS that I think is caused by an unexpected discrepancy in how darwin is referenced (as osx or as macos). In this bug report I outline how it fails and how to make it succeed.

I think that @shelbyspees in https://github.com/wjdp/htmltest/issues/146#issuecomment-818876938 is describing the problem I'm encountering. It's a different problem than that issue originally covered, so I'm opening this issue to isolate that specific problem and to propose a solution.

Looks like on 15 Jan 2021, this PR https://github.com/wjdp/htmltest/pull/157 was merged to implement github CI. There was also a small change to how the darwin releases are identified.

For the releases, darwin: osx was replaced by darwin: macos. This means that the 0.14.0 release in https://github.com/wjdp/htmltest/releases/tag/v0.14.0 (released on 23 Jan 2021) uses https://github.com/wjdp/htmltest/releases/download/v0.14.0/htmltest_0.14.0_macos_amd64.tar.gz for the mac assets.

Unfortunately, we now have a mismatch, because the godownloader.sh script specifies osx in two places: darwin) OS=osx ;; on https://github.com/wjdp/htmltest/blob/master/godownloader.sh#L122 darwin) ARCH=osx ;; on https://github.com/wjdp/htmltest/blob/master/godownloader.sh#L133

I could submit a PR to the godownloader.sh script but it seems you prefer to generate it, and it also seems that this change may have been accidental, as that change from osx to macos doesn't appear to be discussed at all in the issue or PR - it just happened without commentary or discussion.

Suggested fix: either change the name of the released file, or change the script - whichever you choose, making them match will fix this problem.

To Reproduce

Steps to reproduce the behaviour:

Here's the failure I get when testing locally on a mac.

$ curl https://raw.githubusercontent.com/wjdp/htmltest/master/godownloader.sh | bash
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  9932  100  9932    0     0  45559      0 --:--:-- --:--:-- --:--:-- 45351
wjdp/htmltest info checking GitHub for latest tag
wjdp/htmltest info found version: 0.14.0 for v0.14.0/osx/amd64
$ echo $?
1
$

It's failing, but why? I can see more detail if I download the script and run it manually with the debug flag. Looks like we are looking for a file under a name where it does not exist:

$ curl https://raw.githubusercontent.com/wjdp/htmltest/master/godownloader.sh > godownloader.sh
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  9932  100  9932    0     0  80747      0 --:--:-- --:--:-- --:--:-- 80747
$ bash godownloader.sh
wjdp/htmltest info checking GitHub for latest tag
wjdp/htmltest info found version: 0.14.0 for v0.14.0/osx/amd64
$ bash godownloader.sh -d
wjdp/htmltest info checking GitHub for latest tag
wjdp/htmltest debug http_download https://github.com/wjdp/htmltest/releases/latest
wjdp/htmltest info found version: 0.14.0 for v0.14.0/osx/amd64
wjdp/htmltest debug downloading files into /var/folders/zc/z3xkvh3x0p9618p_lrv75l4h0000gn/T/
wjdp/htmltest debug http_download https://github.com/wjdp/htmltest/releases/download/v0.14.0/htmltest_0.14.0_osx_amd64.tar.gz
wjdp/htmltest debug http_download_curl received HTTP status 404
$

The failure is because of the change from osx to macos as explained above. To make the script succeed, I can make it look for the release under the name that currently exists:

$ grep osx godownloader.sh
    darwin) OS=osx ;;
    darwin) ARCH=osx ;;
$ sed -i '' "s/osx/macos/g" godownloader.sh
$ grep osx godownloader.sh
$ grep macos godownloader.sh
    darwin) OS=macos ;;
    darwin) ARCH=macos ;;
$ bash godownloader.sh -d
wjdp/htmltest info checking GitHub for latest tag
wjdp/htmltest debug http_download https://github.com/wjdp/htmltest/releases/latest
wjdp/htmltest info found version: 0.14.0 for v0.14.0/macos/amd64
wjdp/htmltest debug downloading files into /var/folders/zc/z3xkvh3x0p9618p_lrv75l4h0000gn/T/
wjdp/htmltest debug http_download https://github.com/wjdp/htmltest/releases/download/v0.14.0/htmltest_0.14.0_macos_amd64.tar.gz
wjdp/htmltest debug http_download https://github.com/wjdp/htmltest/releases/download/v0.14.0/htmltest_0.14.0_checksums.txt
wjdp/htmltest info installed ./bin/htmltest
$ 

.htmltest.yml

Please copy in your config file

DirectoryPath: app
IgnoreDirectoryMissingTrailingSlash: true
CheckExternal: false
IgnoreAltMissing: true

Source files

My source files aren't relevant to this bug.

Expected behaviour

I expect the released binaries of htmltest to be named in a way that's compatible with the recommended installation procedure.

Actual behaviour

Due to the name mismatch, testing on a mac requires an undocumented additional step of s/osx/macos/g as shown above.

Versions

$ uname -m
x86_64
$ uname -s
Darwin
$ bin/htmltest -v
htmltest 0.14.0
2021-01-23T18:38:31Z
$
wjdp commented 3 years ago

Hi @bridgetkromhout Thanks for the investigation and write-up!

I expect the released binaries of htmltest to be named in a way that's compatible with the recommended installation procedure.

:grin: Love this

It seems I tried to fix this in Jan by re-generating the godownloader script. The copy on htmltest.wjdp.uk is hosted on S3 and I updated this while failing to update the repo copy. The PR linked fixes this.

A silly mistake which has cost a few people time looking into, sorry for this and thanks again for raising.

bridgetkromhout commented 3 years ago

The PR linked fixes this.

Great! Looking forward to seeing that merged. Thanks!