Migrate http/command/file resources to use gomega matchers

[pedroMMM]

Describe the feature:

The http/command/file resources don't support Gomega matchers which have more features than their current pattern matching like described on #532

Describe the solution you'd like

We whole io.Reader shouldn't be dumped to memory in case the contents are huge. The solution is to stream the contents into a custom ContainElement like Gomega matcher, like talked about on #508 (comment).
The backward compatibility with the pattern should be kept.

Describe alternatives you've considered

N/A

[pedroMMM]

@aelsabbahy While you are around working on Goss you mind taking a look at this FR? We have talked about this before but I want to ensure we are on the same page as much as possible since it will be a decent development effort.

[pedroMMM]

Would this also allow for the use of and, or, not clauses in file contents?

I have a challenge of checking some URLs in a config file to test if they are correct and they could either be the IP or hostname. Right now I don't believe it's possible hence my interest.

@donmstewart if those URLs are on the same lines you might be able to do the boolean logic via regex using the Pattern Matcher.

Feel free to open an issue with your use case and example files and we can try to help you.

Please add a 👍 to the original issue.

[aelsabbahy]

The io.reader logic will be a bit tricky to figure out. I'm really curious on your proposed solution @pedroMMM.

In my mind it seems very difficult to have both the reader + complex tests without duplicating the streams, dumping into memory, or other magic.

Wonder which implementation would be easier:

• Support for special io.reader matchers
• Convert reader to array whenever it's using these matchers. Put a note in the doc about losing memory efficiency ("don't do this on a 50gb file")

[pedroMMM]

@aelsabbahy I have been thinking of creating a special io.Reader matcher that on a sliding window the stream converts to an array and then applies the regular []string matcher. The sliding window should then be configurable by the test but default to something like 10 or so.

I am suggesting a sliding window so we can target evaluations that have break lines in them. But I am more than open to default the window to 1 and bypass the slower code so it defaults to a faster evaluation.

Honestly, it's been a while since I looked at the code so I need to freshen up the implementation but that is the essence of my original idea.

[aelsabbahy]

I guess in my ideal would be being able to support all matchers across the board.

So the following can/should be valid:

command:
  echo 2:
    exit-status: 0
    stdout:
      and:
       # Transforms and tests against numeric
       - lt: 5
       # This is either the old pattern matcher (or maybe the new #538 complex matcher/MatchAllElements)
       - NewPatternMatcher:
           - '/[0-5]/'

Let me know if my example doesn't make sense. Basically, I'd like to get it so that all attributes are going through gomege and are always transformed into the proper type before testing.

I'm thinking once that's implemented, it would make it so all the tests behave similarly. It may also alleviate the issue of not being able to see the output of command when it fails as that comes up as a common difficulty with goss.

[pedroMMM]

@aelsabbahy so you are suggesting to go down the simple path of

Convert reader to array whenever it's using these matchers. Put a note in the doc about losing memory efficiency ("don't do this on a 50gb file")

Honestly, I am not opposed to it at all. Keeping things simpler should keep bugs and performance issues at bay.

Let me know if my example doesn't make sense. Basically, I'd like to get it so that all attributes are going through gomege and are always transformed into the proper type before testing.

My biggest worry here is the existence of a breaking change and your example implies it. I am also still not clear on what you have in mind for #538, but discuss it more especially since you have a plan in mind for the order of implementation.

[aelsabbahy]

hmm.. wonder if we can somehow implement it in a backwards compatible way. So keeping the old functionality if an array is passed, but as soon as you use a mapping, string or numeric it's converted to whatever type is the test is of (and loaded in memory).

example:

command:
  echo 2:
    exit-status: 0
    # works same as before
    stdout: ["foo", "/bar/"]
    # converts to string, array, numeric
    stdout: "foo"
    stdout:
        contains-element: ...
    stdout:
        lt: ...

Honestly, I am not opposed to it at all. Keeping things simpler should keep bugs and performance issues at bay.

This is the ideal that I'm trying to get to with #538 and this ticket. I would love for all attributes to behave the same way for the end user, it will make adding any new attributes a breeze and immediately gives a consistent API to the user.

(I'm thinking out loud here, so might be half baked idea)..

Maybe the attributes need to have a concept of a "default matcher?" Unless you can think of a better way to handle the difference between command.stdout (test is an array, but uses io.reader and pattern matching) vs user.groups (test is also an array, but uses contain element) https://github.com/aelsabbahy/goss/blob/master/resource/gomega_test.go#L34

[pedroMMM]

Maybe the attributes need to have a concept of a "default matcher?" Unless you can think of a better way to handle the difference between command.stdout (test is an array, but uses io.reader and pattern matching) vs user.groups (test is also an array, but uses contain element) https://github.com/aelsabbahy/goss/blob/master/resource/gomega_test.go#L34

So here is where I want to break backward compatibility!

command:
  echo 2:
    exit-status: 0
    stdout: 
      # uses the gomega.ContainElement
      - "foo" 
      # uses the gomega.MatchRegexp
      - "/bar/"

Essentially it would force all the users that have the simple pattern "matcher" into adding a regex (/old pattern/). It's a small change but it could make this so much simpler. We could still keep support for the negative regex !/.../ since we have to evaluate each element in the array anyways.

[aelsabbahy]

@pedroMMM This + transforms has been on my mental backlog for a while and I know the verbiage on #538 probably didn't do my thoughts justice.

I've added a comment with an example branch in #538 that provides a rough initial draft of my thoughts + example files.

This has been a major feature I've wanted for a long time and I finally got time to hammer out an initial POC.

If you don't mind, let's continue some of this chat on #538 since all these features are interrelated.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[aelsabbahy]

#576 will close this out. Marking this as a duplicate #538