fixes #318 (I wonder if this works as a comment)

This looks great. Only question I have is whether we can cache the results rather than the rendered output?

That can be a later improvement, since I think it will conflict with my big v4 refactor.

What order should all these PR's be merged in? I'm planning to try to get them all merged this week.

@aelsabbahy

This looks great. Only question I have is whether we can cache the results rather than the rendered output?

Yeah, I'd been thinking about that too. Yes we can. I've filed #612.

re: order of merges - #586, #608, #606 are independent and can go first. I'll then rebase this one. I noticed https://probot.github.io/apps/dep/ which might allow this kind of maintainer-toil to be a bit easier?

Got a chance to test this out today, I'm not sure if I'm misunderstanding the documentation or if there's a bug with the code.

The following don't work and fallback on rspecish

$ curl -H "Accept: application/vnd.goss-documentation" localhost:8080/healthz
$ curl -H "Accept: application/vnd.goss-tap" localhost:8080/healthz

2020/10/02 09:19:40 candidate "application/vnd.goss-documentation"
2020/10/02 09:19:40 Warn: Using process-level output-format. Accept header on request missing or invalid. Accept header: [application/vnd.goss-documentation]
2020/10/02 09:19:40 [::1]:56852: requesting health probe
2020/10/02 09:19:40 Stale cache[res:application/vnd.goss-rspecish], running tests
2020/10/02 09:19:40 exitCode: 0
2020/10/02 09:19:40 [::1]:56852: status 200
2020/10/02 09:20:32 candidate "application/vnd.goss-tap"
2020/10/02 09:20:32 Warn: Using process-level output-format. Accept header on request missing or invalid. Accept header: [application/vnd.goss-tap]
2020/10/02 09:20:32 [::1]:56874: requesting health probe
2020/10/02 09:20:32 Stale cache[res:application/vnd.goss-rspecish], running tests
2020/10/02 09:20:32 exitCode: 0
2020/10/02 09:20:32 [::1]:56874: status 200

Seems only JSON works?

Got a chance to test this out today, I'm not sure if I'm misunderstanding the documentation or if there's a bug with the code.

The following don't work and fallback on rspecish

$ curl -H "Accept: application/vnd.goss-documentation" localhost:8080/healthz
$ curl -H "Accept: application/vnd.goss-tap" localhost:8080/healthz

2020/10/02 09:19:40 candidate "application/vnd.goss-documentation"
2020/10/02 09:19:40 Warn: Using process-level output-format. Accept header on request missing or invalid. Accept header: [application/vnd.goss-documentation]
2020/10/02 09:19:40 [::1]:56852: requesting health probe
2020/10/02 09:19:40 Stale cache[res:application/vnd.goss-rspecish], running tests
2020/10/02 09:19:40 exitCode: 0
2020/10/02 09:19:40 [::1]:56852: status 200
2020/10/02 09:20:32 candidate "application/vnd.goss-tap"
2020/10/02 09:20:32 Warn: Using process-level output-format. Accept header on request missing or invalid. Accept header: [application/vnd.goss-tap]
2020/10/02 09:20:32 [::1]:56874: requesting health probe
2020/10/02 09:20:32 Stale cache[res:application/vnd.goss-rspecish], running tests
2020/10/02 09:20:32 exitCode: 0
2020/10/02 09:20:32 [::1]:56874: status 200

Seems only JSON works?

@aelsabbahy - you're not wrong 🤦 . Fixed in cf00396.

Lol, all good. I tend to manually test most PRs just in case.

Thanks for updating the tests as part of this. On my end I'm planning to:

1. Run the old code with new tests to make sure I fails.
2. Manually test new code

Assuming both of those succeed, I'll merge this PR and probably cut a release.

That'd be great. What're the chances of #611 making it in, even if #607 doesn't yet? (Although 607 seems to have attained ship-then-iterate consensus, I think maybe)

I'll review #611 I was indifferent about it merging before the release since it seems to be just an internal refactor.

#607 I'm giving others a week to speak or forever hold their peace.

Is 607 dependent on 611? If so, I can just wait a week, merge all three and release.

#611 is a refactor that will enable a hack in #607 to be removed; the order they land doesn't make much difference, I'll update whichever the 2nd one is to make good.

Tested this and it looks great. It's a little chatty (on the serve side) and I was wondering if that was intentional or leftover debug messages?

@petemounce ping (only pinging you since I noticed you pushed out a change to the other PR)

@aelsabbahy whoops, yes; those were debugs. I thought it was valuable to know when a response was coming from cache vs being calculated, so I left those in.

I don't think it's particularly feasible to prefix the referer to ever log message at present using only the facilities of the log package, or by complicating the code to pass the values around up and down the callstack, so I didn't do that. If we want that, I think the best course is to use a log library with those sorts of features out-of-the-box.

Last one, is it intentional to have the test result in the log data of the serve command when test is failing?

Here's what I see when a run fails with "documentation":

$ goss serve
2020/10/09 10:01:04 Starting to listen on: :8080
2020/10/09 10:01:08 [::1]:46034: requesting health probe
2020/10/09 10:01:08 Stale cache[res:application/vnd.goss-documentation], running tests
2020/10/09 10:01:08 [::1]:46034: status 503 - Package: kernel: installed: matches expectation: [true]
Package: kernel: version:
Expected
    <[]string | len:3, cap:3>: ["4.18.7", "4.18.16", "4.18.19"]
to contain element matching
    <string>: 4.18.3


Failures/Skipped:

Package: kernel: version:
Expected
    <[]string | len:3, cap:3>: ["4.18.7", "4.18.16", "4.18.19"]
to contain element matching
    <string>: 4.18.3

Total Duration: 0.009s
Count: 2, Failed: 1, Skipped: 0

Here's a success:

$ goss serve
2020/10/09 10:02:07 Starting to listen on: :8080
2020/10/09 10:02:09 [::1]:46046: requesting health probe
2020/10/09 10:02:09 Stale cache[res:application/vnd.goss-documentation], running tests
2020/10/09 10:02:09 [::1]:46046: status 200

Yes, that was intentional.

When a test fails, without log output the options an operator has to debug it are to remote onto the machine and run it again.

If the test fails intermittently, this will be harder to debug and reproduce. If it's time dependent, the operator may never be awake.

Tests that are hard to debug, in my experience, are removed or worse, ignored, devaluing the other tests by association.

Awesome work! Thank you for all the time and effort you've put into contributing to goss!

My pleasure, thanks for merging :)