request for feedback and help with bug

[GETandSELECT]

Hi

request Feedback

this is my first formula in homebrew non-cask

what can I improve?

class BlackboxExporter < Formula
  desc "Blackbox prober exporter"
  homepage "https://prometheus.io"
  url "https://github.com/prometheus/blackbox_exporter/archive/v0.18.0.zip"
  sha256 "a0b5949a0266d4fb6761b54c73efd61ae0dbdc611dc5be7e66aad20c6cf7eee1"
  license "Apache-2.0"
  head "https://github.com/prometheus/blackbox_exporter.git"

  livecheck do
    url :head
    regex(/^v?(\d+(?:\.\d+)+)$/i)
  end

  depends_on "go" => :build

  def install
    ldflags = %W[
      -X github.com/prometheus/common/version.Version=#{version}
      -X github.com/prometheus/common/version.BuildUser=Homebrew
    ]
    system "go", "build", "-ldflags", ldflags.join(" "), "-trimpath",
           "-o", bin/"blackbox_exporter"
    prefix.install_metafiles

    mkdir etc/"blackbox_exporter"

    (etc/"blackbox_exporter.args").write <<~EOS
      --config.file /usr/local/etc/blackbox_exporter/blackbox.yml
    EOS

    (etc/"blackbox_exporter/blackbox.yml").write <<~EOS
modules:
  http_2xx:
    prober: http
    timeout: 5s
    http:
      valid_status_codes: []
      method: GET
    EOS

    (bin/"blackbox_exporter_brew_services").write <<~EOS
      #!/bin/bash
      exec #{bin}/blackbox_exporter $(<#{etc}/blackbox_exporter.args)
    EOS
  end

  def caveats
    <<~EOS
      When run from `brew services`, `blackbox_exporter` is run from
      `blackbox_exporter_brew_services` and uses the flags in:
        #{etc}/blackbox_exporter.args
    EOS
  end

  plist_options manual: "blackbox_exporter"

  def plist
    <<~EOS
      <?xml version="1.0" encoding="UTF-8"?>
        <!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
        <plist version="1.0">
        <dict>
          <key>Label</key>
          <string>#{plist_name}</string>
          <key>ProgramArguments</key>
          <array>
            <string>#{opt_bin}/blackbox_exporter_brew_services</string>
          </array>
          <key>RunAtLoad</key>
          <true/>
          <key>KeepAlive</key>
          <false/>
          <key>StandardErrorPath</key>
          <string>#{var}/log/blackbox_exporter.err.log</string>
          <key>StandardOutPath</key>
          <string>#{var}/log/blackbox_exporter.log</string>
        </dict>
      </plist>
    EOS
  end

  test do
    assert_match /blackbox_exporter/, shell_output("#{bin}/blackbox_exporter --version 2>&1")

    fork { exec bin/"blackbox_exporter" }
    sleep 2
    assert_match "# HELP", shell_output("curl -s localhost:9115/metrics")
  end
end

Bug

some files don't get deleted at uninstallation

cleanup (I not like this step)

rm -rf /usr/local/etc/blackbox_exporter.args /usr/local/etc/blackbox_exporter

install

brew install blackbox_exporter
==> Downloading https://github.com/prometheus/blackbox_exporter/archive/v0.18.0.zip
(..)
==> Summary
🍺  /usr/local/Cellar/blackbox_exporter/0.18.0: 8 files, 17.3MB, built in 7 seconds

uninstall not OK

brew uninstall blackbox_exporter
Uninstalling /usr/local/Cellar/blackbox_exporter/0.18.0... (8 files, 17.3MB)

Warning: The following blackbox_exporter configuration files have not been removed!
If desired, remove them manually with `rm -rf`:
  /usr/local/etc/blackbox_exporter
  /usr/local/etc/blackbox_exporter/blackbox.yml

Warning: The following may be blackbox_exporter configuration files and have not been removed!
If desired, remove them manually with `rm -rf`:
  /usr/local/etc/blackbox_exporter.args

[SMillerDev]

It's hard to give feedback on code in a text-only platform like discussions, pull requests are a better alternative. I think you misunderstand the warning though. Brew will warn you that files are left behind so you can clean them up if needed, it doesn't mean that there is anything wrong with your code. Files in #{etc} are config files and are intentionally not removed by brew, it'll just warn you about them.

[GETandSELECT]

Before I do a PR, I saw that the test is not passing:

	  test do
	    assert_match /Config file is ok/, shell_output("#{bin}/blackbox_exporter --config.file /usr/local/etc/blackbox_exporter/blackbox.yml --config.check 2>&1")

	    fork { exec bin/"blackbox_exporter" }
	    sleep 2
	    assert_match "# HELP", shell_output("curl -s localhost:9115/metrics")
	  end
	end

here not OK

brew test blackbox_exporter
==> Testing blackbox_exporter
==> /usr/local/Cellar/blackbox_exporter/0.18.0/bin/blackbox_exporter --config.file /usr/local/etc/blackbox_exporter/blackbox.yml --config.check 2>&1
level=info ts=2020-11-09T14:49:12.268Z caller=main.go:212 msg="Starting blackbox_exporter" version="(version=0.18.0, branch=, revision=)"
level=info ts=2020-11-09T14:49:12.268Z caller=main.go:213 msg="Build context" (gogo1.15.4,userHomebrew,date)=(MISSING)
level=error ts=2020-11-09T14:49:12.268Z caller=main.go:216 msg="Error loading config" err="error reading config file: open blackbox.yml: no such file or directory"
==> curl -s localhost:9115/metrics
*   Trying 127.0.0.1...
* TCP_NODELAY set
* Connected to localhost (127.0.0.1) port 9115 (#0)
> GET /metrics HTTP/1.1
> Host: localhost:9115
> User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:68.0) Gecko/20100101 Firefox/68.0
> Accept: */*
> Referer:
>
< HTTP/1.1 200 OK
< Content-Type: text/plain; version=0.0.4; charset=utf-8
< Date: Mon, 09 Nov 2020 14:49:14 GMT
< Transfer-Encoding: chunked
<
{ [5869 bytes data]
* Connection #0 to host localhost left intact
* Closing connection 0

but the same copy+paste on CLI working

/usr/local/Cellar/blackbox_exporter/0.18.0/bin/blackbox_exporter --config.file /usr/local/etc/blackbox_exporter/blackbox.yml --config.check
level=info ts=2020-11-09T14:49:36.083Z caller=main.go:212 msg="Starting blackbox_exporter" version="(version=0.18.0, branch=, revision=)"
level=info ts=2020-11-09T14:49:36.083Z caller=main.go:213 msg="Build context" (gogo1.15.4,userHomebrew,date)=(MISSING)
level=info ts=2020-11-09T14:49:36.084Z caller=main.go:221 msg="Config file is ok exiting..." 

why?

[jonchang]

It's hard to give feedback on code in a text-only platform like discussions, pull requests are a better alternative. Without the full output of the brew test command it's hard to understand what could be going wrong. My guess is that shell_output is getting an error code that you're not checking when you type it into the command line. See https://rubydoc.brew.sh/Homebrew/Assertions.html#shell_output-instance_method for details.

[GETandSELECT]

Changed now (removed sdt out and error redirection 2>&1)

tail -7 blackbox_exporter.rb
	     assert_match /blackbox_exporter,/, shell_output("#{bin}/blackbox_exporter --config.file #{etc}/blackbox_exporter/blackbox.yml --version")

	    fork { exec bin/"blackbox_exporter" }
	    sleep 2
	    assert_match "# HELP", shell_output("curl -s localhost:9115/metrics")
	  end
	end

and get a different error

brew test blackbox_exporter
==> Testing blackbox_exporter
==> /usr/local/Cellar/blackbox_exporter/0.18.0/bin/blackbox_exporter --config.file /usr/local/etc/blackbox_exporter/blackbox.yml --version
blackbox_exporter, version 0.18.0 (branch: , revision: )
  build user:       Homebrew
  build date:
  go version:       go1.15.4
Error: blackbox_exporter: failed
An exception occurred within a child process:
  Test::Unit::AssertionFailedError: </blackbox_exporter,/> was expected to be =~
<"">.
/Users/yy/.gem/ruby/2.6.0/gems/test-unit-3.3.6/lib/test/unit/assertions.rb:59:in `block in assert_block'
/Users/yy/.gem/ruby/2.6.0/gems/test-unit-3.3.6/lib/test/unit/assertions.rb:1635:in `_wrap_assertion'
/Users/yy/.gem/ruby/2.6.0/gems/test-unit-3.3.6/lib/test/unit/assertions.rb:53:in `assert_block'
/Users/yy/.gem/ruby/2.6.0/gems/test-unit-3.3.6/lib/test/unit/assertions.rb:546:in `block in assert_match'
/Users/yy/.gem/ruby/2.6.0/gems/test-unit-3.3.6/lib/test/unit/assertions.rb:1640:in `_wrap_assertion'
/Users/yy/.gem/ruby/2.6.0/gems/test-unit-3.3.6/lib/test/unit/assertions.rb:536:in `assert_match'
/usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula/blackbox_exporter.rb:84:in `block in <class:BlackboxExporter>'
/usr/local/Homebrew/Library/Homebrew/formula.rb:1858:in `block (3 levels) in run_test'
/usr/local/Homebrew/Library/Homebrew/utils.rb:497:in `with_env'
/usr/local/Homebrew/Library/Homebrew/formula.rb:1857:in `block (2 levels) in run_test'
/usr/local/Homebrew/Library/Homebrew/formula.rb:903:in `with_logging'
/usr/local/Homebrew/Library/Homebrew/formula.rb:1856:in `block in run_test'
/usr/local/Homebrew/Library/Homebrew/mktemp.rb:58:in `block in run'
/usr/local/Homebrew/Library/Homebrew/mktemp.rb:58:in `chdir'
/usr/local/Homebrew/Library/Homebrew/mktemp.rb:58:in `run'
/usr/local/Homebrew/Library/Homebrew/formula.rb:2099:in `mktemp'
/usr/local/Homebrew/Library/Homebrew/formula.rb:1850:in `run_test'
/usr/local/Homebrew/Library/Homebrew/test.rb:43:in `block in <main>'
/System/Library/Frameworks/Ruby.framework/Versions/2.6/usr/lib/ruby/2.6.0/timeout.rb:93:in `block in timeout'
/System/Library/Frameworks/Ruby.framework/Versions/2.6/usr/lib/ruby/2.6.0/timeout.rb:33:in `block in catch'
/System/Library/Frameworks/Ruby.framework/Versions/2.6/usr/lib/ruby/2.6.0/timeout.rb:33:in `catch'
/System/Library/Frameworks/Ruby.framework/Versions/2.6/usr/lib/ruby/2.6.0/timeout.rb:33:in `catch'
/System/Library/Frameworks/Ruby.framework/Versions/2.6/usr/lib/ruby/2.6.0/timeout.rb:108:in `timeout'
/usr/local/Homebrew/Library/Homebrew/test.rb:42:in `<main>'

[gromgit]

It looks like the output you're trying to match is being sent to stderr, which is not returned by shell_output. You probably want to redirect stderr to stdout in your command invocation, like this:

	     assert_match /blackbox_exporter,/, shell_output("#{bin}/blackbox_exporter --config.file #{etc}/blackbox_exporter/blackbox.yml --version 2>&1")