Add option to disable error status code printing

[JacobEvelyn]

PR #688 added functionality that broke the
JacobEvelyn/friends
build (see https://travis-ci.com/JacobEvelyn/friends/jobs/238680478),
which runs many CLI processes and parses the output. Some CLI
processes are expected to have non-success status codes, but
these tests also check that our CLI wrote the correct message
to STDERR, and with #688 the output to STDERR is now changed.

This commit adds a SimpleCov configuration flag to disable the
message added in #688.

[PragTob]

@JacobEvelyn Hi there and thanks for the PR! 💚

Might I ask - can't you just adjust your tests to deal with the new printed exit status? Or ideally, not have simplecov fail and exit with an that error message?

I'm open to changing the message to be potentially more parsable, but the reasoning for disabling it I don't understand right now (see questions above).

[JacobEvelyn]

Good questions, @PragTob! I definitely considered that, but it seemed a little hacky to me.

Context that might be helpful

How friends works

friends is a CLI Ruby app.

Typically, if you run friends <arguments> in your shell it'll do something along the lines of:

1. Read a file.
2. Write to the file.
3. Print a message to STDOUT.
4. Exit with a 0 status code.

And if you run friends <bad arguments> it'll do something along the lines of:

1. Print a message to STDERR.
2. Exit with a non-0 status code.

How friends is tested (+ simplecov)

The tests we use can be thought of as "black-box" integration tests; rather than test some internals of its implementation, each test in our Minitest suite actually creates a new process and runs friends <whatever> and then uses things like Open3 and File.read to see the process' output (including STDOUT, STDERR, and status code) and any file changes it made.

This testing setup simplifies a lot of things, but means that to get code coverage to work we have to have code in the CLI itself (the sub-process that each test is executing) that starts up simplecov (it uses SimpleCov.command_name Process.pid.to_s so each test doesn't wipe out the other tests' coverage data).

The problem this PR is solving

What's happening here is for our tests that are testing bad input (the friends <bad arguments> cases), when simplecov is enabled the tests fail because they're expecting STDERR to say something like Invalid argument: <bad argument> and instead they now say:

Invalid argument: <bad argument>
SimpleCov failed with exit 1

Alternatives to this PR

The way I see it, there are three obvious options to fixing our issue in friends:

1. Change friends tests to allow simplecov's output

This is I believe what you're suggesting, and while I did consider it I decided to not pursue this for a few reasons:

1. friends is suddenly tied very closely to a specific output message of simplecov, which of course might change in the future and break our tests again. This feels a little hacky to me.
2. It's not entirely straightforward to do this, since we run tests both with and without simplecov. We'd need to add the complexity of ignoring the simplecov message if it's present but not depending on it being there.
3. Any other simplecov users out there who might run into the same issue will need to roll their own solutions.

2. Add a configuration option that allows disabling the #688 message

This seemed relatively simple and straightforward, and is this PR.

3. Add a configuration option that allows global changes to simplecov output

For example, a quiet mode that silences all output, or a way to direct all simplecov output to a file. This seemed like a much more ambitious PR for me to make, and I don't know how useful others would find the functionality. We also do like some of simplecov's output, so a global configuration would be less useful for us.

---

Sorry for the lengthy comment! Does all of that make sense? Thoughts?

[PragTob]

@JacobEvelyn thanks a lot for the lengthy explanation, a bit more than I wanted but it helps turnaround time here! 😁 👍

Yeah the most important for me is the starting of sub processes that have to have simplecov enabled themselves in the black box tests. Although a bit odd I can see how it's a very valid approach (we're actually doing the same kinda just with the difference that we want to test simplecov).

I agree it's the simplest solution, supporting it going forward shouldn't be too complicated so I'm fine with having it 🎉

It'd be great though if you could add tests for it and document it in the README. If not, no worries but you'll have to wait for me to get around to doing it :)

[JacobEvelyn]

Sure, I'll add some tests and documentation. Thanks for talking it through with me!

[JacobEvelyn]

Okay @PragTob! I just added more changes to my commit here to put documentation in the README and add tests. Let me know what you think!

[PragTob]

Thanks!

Those are even more tests than I would have wanted, I'd have probably just written the 2 unit tests and then one cucumber feature but it's also good like this.

Thanks for taking the time and contributing! 💚

[JacobEvelyn]

Thanks so much, @PragTob! Do you have a sense for when the next release will be?

[PragTob]

@JacobEvelyn not right now no, some of it depends on whether or not I get rubytogether funding to work on this. I'll try and re evaluate it this weekend, technically there's barely such a thing as too small releases. Would it be a big bother for you to use it from github in the mean time?

[JacobEvelyn]

Unfortunately, since I'm using this in a gem I can't use it from GitHub because there's no way to specify GitHub dependencies in gemspec files. 😢 So sooner is better for me, but as a fellow OSS maintainer I know it's not always easy to find the time for these things—please don't feel pressure from me to rush!

[PragTob]

@JacobEvelyn you don't need to have development dependencies in the gemspec, you can just add them in the Gemfile as a normal gem entry and can then use github and what not :)

[JacobEvelyn]

Ah neat, thanks!