speedread 2016.01.10 (new formula)

[hashier]

A simple terminal-based open source Spritz-alike.

This command line filter shows input text as a per-word RSVP (rapid
serial visual presentation) aligned on optimal reading points. This kind
of input mode allows reading text at a much more rapid pace than usual
as the eye can stay fixed on a single place.

[bfontaine]

We need a tagged release here.

[hashier]

I asked the upstream maintainer to add a version tag. The last commit to the project is 2014 so I don't know whether this is gonna happen.

[hashier]

Changing the test to what @bfontaine recommended ends with travis not being able to run the test and complaining about tty errors. Locally the test runs just fine.

$ brew test speedread
Testing speedread
==> Using the sandbox
==> /usr/local/Cellar/speedread/2016.01.10/bin/speedread -w 1000
Use of the encoding pragma is deprecated at /usr/local/Cellar/speedread/2016.01.10/bin/speedread line 39.
Real: 8.42s User: 4.20s System: 1.50s Percent: 67% Cmd: brew test speedread

[bfontaine]

@BrewTestBot test this please.

[dunn]

The test is still erroring out, at least on Travis: https://travis-ci.org/Homebrew/homebrew/jobs/101895620

[hashier]

@bfontaine I changed all the things you wanted (tagged release and test) but now it doesn't build on Travis anymore (though the test success locally). What should I do now?

[bfontaine]

You can remove this line; Homebrew detects it from the URL 😉

[hashier]

Done. What about the test now?

[bfontaine]

Try adding a newline at the end of the test string: "This is a test\n". Maybe speedread waits for a full line, and the original test did have a newline (echo adds a newline by default).

[hashier]

@BrewTestBot test this please.

[bfontaine]

@hashier It won’t work if you’re not a maintainer 😉
The build has been triggered but there are some PRs before it in the queue so we have to wait.

[hashier]

@MikeMcQuaid / @bfontaine What shall I do? Travis clearly has a problem building this formula.

[bfontaine]

@hashier You can try prefixing the test command with script -q /dev/null. This usually fixes this kind of issue:

    pipe_output("script -q /dev/null #{bin}/speedread -w 1000", "This is a test\n")

[hashier]

@bfontaine That doesn't even finish/work/passes the test locally

[hashier]

Well, I'll push it now anyway sigh

[bfontaine]

@hashier Sorry about the delay. I was able to get a working test like so:

  test do
    ENV["TERM"] = "xterm"
    pipe_output("#{bin}/speedread -w 1000", "This is a test\n")
  end

[DomT4]

The test is hanging indefinitely here on CI. Build finishes inside ~20 minutes and then for the next 100 minutes it gets stuck on brew test --verbose speedread

[bfontaine]

@DomT4 Yeah we often get this issue with CLI tools that require user input. Setting TERM and/or using script -q /dev/null usually fix it; the test block I posted should work.

You can reproduce the CI error by doing the following:

% ssh localhost
$ unset TERM
$ brew test speedread
$ exit
%

(using two prompts to differenciate between the current and the ssh sessions.)

[hashier]

And why is this test "better" than the one I had in the very first pull request? Which didn't need any environment hacks at all to work.

[DomT4]

Still seems to be hanging indefinitely sadly. Aborted after 70 minutes to give the queue some opportunity to make progress.

[bfontaine]

@hashier The original test used something like system "#{bin}/speedread <(echo foo) if I remember correctly, which should be equivalent to pipe_output("#{bin}/speedread", "foo\n"). In this case it’s not, probably because of the interactive nature of the tool.

There’s an upstream issue about that: pasky/speedread#14

[hashier]

If these are (supposedly) equivalent why can't we just use the first one that works? (:

I understand it's nice to fix bugs but it's even nicer to ship something if you can it working and then care about the bug.
(And the given old test is better than just testing for existence which is mentioned in the docs is a bad test but an accepted one).

Sent from my iPhone

On 25 Jan 2016, at 22:51, Baptiste Fontaine notifications@github.com wrote:

@hashier The original test used something like system "#{bin}/speedread <(echo foo) if I remember correctly, which should be equivalent to pipe_output("#{bin}/speedread", "foo\n"). In this case it’s not, probably because of the interactive nature of the tool.

There’s an upstream issue about that: pasky/speedread#14

—
Reply to this email directly or view it on GitHub.

[bfontaine]

@hashier I originally thought these were equivalent but I was proven wrong: the first one spawns an interactive shell while the second one doesn’t.

I understand it's nice to fix bugs but it's even nicer to ship something if you can it working and then care about the bug.

That’s true in some contexts; but I don’t think it applies here. Homebrew is a package manager with a very limited number of maintainers. We get issues when things are broken, so shipping a formula for something with a (known) bug is not a good strategy if we want to optimize our time. I, however, agree it’s a minor bug because it doesn’t occur in normal (i.e. interactive) usage.

(And the given old test is better than just testing for existence which is mentioned in the docs is a bad test but an accepted one).

Note that the new test doesn’t only test for existence, it’s executing speedread -w 1000 by piping input to it. pipe_output uses IO.popen under the hood.

cc @DomT4 @MikeMcQuaid for other opinions.

[MikeMcQuaid]

I'm OK with any test that's green on both continuous-integration/travis-ci/pr and default builds.

[bfontaine]

@hashier You can reuse your first test; I’ll try the pipe_output thing later.

[hashier]

Just needed some time to find the old commit in $ git reflog will push later. Github seems to block my push from a mobile IP

[bfontaine]

I can see it was pushed (and passed the tests). Thank you for your contribution to Homebrew! 🎉

[hashier]

Thanks @bfontaine with your patience as well (: Hope you manage to find the bug in the test.